fix: follow ups and QoL improvements - #28
Draft
raisedadead wants to merge 15 commits into
Draft
Conversation
raisedadead
marked this pull request as draft
July 31, 2026 16:47
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-ups from the #27 review, plus one pre-existing bug and some CI wiring.
formatHintOutputre-parsed HTML in order to escape it. A parser discards what it doesn't model before escaping can preserve it as text, and applies content models that don't fit a plain-text hint. One cause, three defects:Your <img src="cat.jpg"> is missing alt.Your <img /> …— attributes droppedYour <img src="cat.jpg"> …Wrap it in <code><textarea></code> and try.<code>…</code> and try.</code>— swallowed closing tag<code><textarea></code> and try.Comments look like <!-- this -->.Comments look like .— comment deletedComments look like <!-- this -->.Tags are now found in the raw output and everything between them is escaped. Detecting tags before escaping is what stops the renderer mistaking model-written
<code>for markup it produced itself. Drops thesanitize-htmldependency.Fixes
<stu<student_code>dent_code>reassemble into<student_code>.challengeTypeguard restored — unrecognised values map toundefinedagain, as before Refactor/stabilize hint contract #27. The return type and docstring both already claimed this. No HTTP-visible change.MAX_HINT_RESPONSE_CHARSis derived (≤5 chars per escaped code point), matching the measured worst case. Documents element-context-only insertion, since"/'aren't escaped. Fastify doesn't enforce responsemaxLength; this is for API consumers.test:manualno longer fails on a healthy dev server — its 401/403 cases can't pass outside production/staging yet gated the exit code, and spent two Groq calls doing it. The runner now detects whether auth is enforced and skips them when it isn't.src/index.ts) — pre-existing, not from Refactor/stabilize hint contract #27. Boot failures were logged aterrorthen dropped before the transport drained.CI and tooling
pnpm run verify(check && typecheck && test) is now the single source of truth; the CI workflow calls it. Local and CI can no longer drift — this PR's first CI failure was exactly that drift,pnpm lintlocally vspnpm run checkin CI, so prettier never ran.typechecknow coversscripts/viatsconfig.check.json(noEmit);tsconfig.jsonstays the build config anddist/is unchanged.src/**/__tests__stays excluded — vitest's axios mocking and dynamicimport()don't satisfymoduleResolution: nodenext, so including them means rewriting mocks, not fixing bugs..scratchpad/dist/coverage, matching.prettierignore. Local file count now matches CI's.CLAUDE.mdupdated throughout: #27 moved auth toonRequest, inverting the documented hook order.Verification
pnpm run verify— 104 tests (up from 88), lint and prettier clean, typecheck clean;pnpm run buildstill emits the Lua script. Every behaviour change written test-first. Verified live against a dev server: contract runner exits 0; four/hintcalls across html/css/javascript returned balanced<code>and no active non-codemarkup.The
chore:/ci:commits are independent of thefix:ones and can be cherry-picked out if you'd rather keep this PR purely about the hint formatting.