experimental switch from npm to bun and upgrade to ts7

[yangm2]

What type of PR is this? (check all applicable)

• Refactor
• Feature
• Bug Fix
• Optimization
• Documentation Update
• Infrastructure
• Maintenance

Description

• replace node/npm with bun
• upgraded to Typescript7
• switched from eslint to oxlint (ts7 compatible)
  • type-aware typechecking found a few more issues
  • enable rules-of-hooks equivalent
  • enable react-refresh/only-export-components equivalent

Related Tickets & Documents

• Related Issue #
• Closes #

QA Instructions, Screenshots, Recordings

Please replace this line with instructions on how to test your changes, a note on the devices and browsers this has been tested on, as well as any relevant images for UI changes.

Added/updated tests?

• Yes
• No, and this is why: tool swap & version bumps
• I need help with writing tests

Documentation

• If this PR changes the system architecture, Architecture.md has been updated

[optional] Are there any post deployment tasks we need to perform?

[yangm2]

@claude code-review

[yangm2]

@claude code-review

[yangm2]

@claude code-review

[yangm2]

@claude code-review

[claude]

Claude finished @yangm2's task in 7m 48s —— View job

---

Code review: bun + TS7 + oxlint swap

• Gather context (diff vs origin/main, CI status)
• Review tooling swap (bun, oxlint, TS7, package.json, mise, Docker, CI)
• Review source changes (type guards, hooks, components)
• Review test changes
• Post review

Overall this is a clean, well-commented migration — SHA-pinned setup-bun, exact-pinned typescript@7.0.2, prettier --check in CI instead of the old prettier --write (which silently mutated the checkout), and the .oxlintrc.json re-enables the rules-of-hooks / only-export-components equivalents as promised. CI is green on 8d3cbdb. A handful of things below, roughly in severity order.

---

1. guards.ts: the "compile-time assertion" doesn't assert anything

frontend/src/shared/types/guards.ts:18-30

export type AssertChunkTypesExhaustive = AssertEqual<
  keyof typeof responseChunkTypes,
  ChunkType
>;

The comment says it "fails to typecheck if … diverge in either direction", but a type alias that resolves to never is perfectly legal TypeScript — nothing consumes this alias, so tsc never complains. If ChunkType gained citation, this line would quietly become type AssertChunkTypesExhaustive = never and build fine.

The Record<ChunkType, true> annotation above does catch both directions on its own (missing key → error, excess property → error), so the current safety net is real — it's just this extra decoupled check that is dead code. To make it live:

type AssertTrue<T extends true> = T;
type _AssertChunkTypesExhaustive = AssertTrue<
  AssertEqual<keyof typeof responseChunkTypes, ChunkType>
>;

(then never violates the extends true constraint). Fix this →

2. Unrecognized chunk types now dump raw JSON into the chat

frontend/src/pages/Chat/components/MessageContent.tsx:93-115

Before: a line that parsed as JSON but wasn't a known chunk reached RenderedChunk, hit default: and rendered null (invisible). Now parseResponseChunk returns null for it and it falls into the markdown branch, so the user sees {"type":"unknown_type","content":"hello"} verbatim — that's exactly what the new test at MessageContent.test.tsx:89-96 locks in.

The markdown fallback was there for non-JSON text. For "valid JSON object, unknown/new type" the old behavior (render nothing) is friendlier — otherwise adding a chunk variant to schema.py before the frontend's generated types catch up puts raw JSON in front of tenants. Risk is low here because both halves deploy from the same commit, but consider distinguishing the two failure modes: null for parsed-but-unrecognized objects, markdown only for JSON.parse throwing. Fix this →

3. FeedbackModal behavior changes (untested, and one UX regression)

frontend/src/pages/Chat/components/FeedbackModal.tsx:55-84

• Lost success confirmation. The old code showed "Feedback Sent!" for the 1s timeout window; now success closes the dialog immediately and the only message a user ever sees is "Sending feedback…". Silent close reads as "did that work?"
• Send isn't disabled while status === "sending". Two clicks → two sendFeedback calls → two feedback emails with two transcript uploads. Pre-existing (the old setTimeout had the same hole), but this PR rewrote the handler, so it's a good moment to add disabled={status === "sending"}.
• Error is swallowed — .catch(() => setStatus("error")) drops the reason; a console.error would help debugging.
• In the error state the textarea is unmounted, so "Please try again" gives the user nothing to try again with (the text is still in state, so Send works, but they can't see or edit it). Returning to idle with an inline error banner would be better.
• There's no FeedbackModal.test.tsx. feedbackHelper got a nice !response.ok test, but the new component-level error branch is untested. The PR checklist says "no tests, tool swap & version bumps" — this file (plus InputField's prop change and item 2) are genuine behavior changes, worth reflecting in the description.

Fix this →

Credit where due: the if (feedback.trim() === "") { handleModalClose(); return; } early return fixes a real old bug (it used to close and then submit empty feedback).

4. The two deploy workflows rely on exactly what pr-check says not to rely on

pr-check.yml gained this comment and step:

json2ts … has a #!/usr/bin/env node shebang; declare the Node dependency explicitly instead of relying on the runner image happening to ship one.

But deploy.staging.yml / deploy.production.yml dropped setup-node entirely and still run bun run build → generate-types → json2ts, i.e. they now depend on the ambient runner Node. Either add the same explicit Node setup there or drop the reasoning from pr-check. The deploys also lost their dependency cache (the old setup-node had cache: npm) — pr-check got an actions/cache for ~/.bun/install/cache, deploys got nothing.

5. actions/setup-node@v5 with no node-version is close to a no-op

pr-check.yml:135 — with no node-version input and no .nvmrc/.node-version file, setup-node doesn't install anything; it warns and leaves the runner's preinstalled Node in place. So the step doesn't actually pin the Node that json2ts runs under, which was its stated purpose. It's also a downgrade to @v5 while checkout/upload-artifact in the same file are on v6/v4. Suggest node-version: 24 (matching frontend/mise.toml) or dropping the step.

Heads-up: I can't push edits to .github/workflows/** myself (GitHub App workflow permissions), so items 4 and 5 need a human commit.

6. .oxlintrc.json ignores dist and src/types but not coverage

frontend/.oxlintrc.json:3. .prettierignore correctly lists coverage/, and frontend/coverage is gitignored, but after a local bun run test:coverage the v8 HTML reporter drops prettify.js / sorter.js / block-navigation.js in coverage/ — and lint runs with --max-warnings=0, so bun run lint will start failing on report assets. Adding "coverage" to ignorePatterns keeps host and CI behavior aligned. Fix this →

7. Three different Node workarounds, and one misleading comment

frontend/mise.toml:6 says node is "Kept for editor/tooling compatibility; frontend/Dockerfile now runs on bun" — but on the host it's load-bearing: bun run generate-types shells out to json2ts, whose shebang is #!/usr/bin/env node. Without mise's node on PATH that task fails, which is a stronger reason than "editor compatibility".

More broadly there are now three parallel patches for the same shebang: mise's node = "24", setup-node in pr-check, and two ln -s $(command -v bun) /usr/local/bin/node calls in the Dockerfile. Worth verifying whether Bun's own node shim (bun run prepends a node→bun symlink to PATH for package scripts) already covers json2ts — if it does, most of this can go; if it doesn't, it'd be good to say so in one place and reference it from the others.

8. Unrelated backend/uv.lock churn

backend/uv.lock flips name = "tenant-first-aid" → "tenantfirstaid" and drops the pexpect platform markers. Nothing under backend/pyproject.toml changed in this PR, so this looks like a drive-by regeneration by a locally-different uv. Note that backend/mise.toml pins uv = "0.11.26" while the workflows pin setup-uv to 0.10.0 — a lock written by the newer resolver being consumed by the older one is the kind of skew that bites later. CI is green, so it resolves today; worth confirming it was intentional (or splitting it out).

9. Nothing stops an accidental npm install

frontend/package.json has no "packageManager" field and package-lock.json isn't in .gitignore, so one stray npm install silently reintroduces the 6.4k-line lockfile this PR just deleted. Adding "packageManager": "bun@1.3.14" (also documents the version next to the Dockerfile/mise/CI pins) and a .gitignore entry for frontend/package-lock.json would make the migration sticky. Fix this →

Nits

• frontend/src/tests/components/MessageWindow.test.tsx:4: import { default as MessageWindow } from … is just import MessageWindow from … with extra ceremony, and it's inconsistent with every other test file. If a lint rule forced it, a comment saying which one would help.
• frontend/src/tests/components/HousingContext.test.tsx:6: hoisting renderOutside to module scope (above the ContextDump it renders) puts it ~45 lines from its single use. Tightening the assertion to the actual error message is a nice improvement though.
• .claude/skills/frontend.md lists the bun commands but not the new format:check or mise run //frontend:clean.
• The CodeQL comments about unpinned oven-sh/setup-bun@v2 are already resolved at 8d3cbdb (all three workflows use 0c5077e… with a #v2.2.0 comment) — they should stop reappearing on the next scan.

Things I liked

• InputField's onChange → setValue removes a genuinely nasty synthetic-ChangeEvent cast rather than just silencing the lint rule.
• feedbackHelper now checks response.ok — that was a real swallowed-failure bug, and it has a test.
• isResponseChunk using a Set over Object.keys specifically to dodge prototype keys, with the reasoning written down.
• --max-warnings=0 plus format:check in CI means the "suspicious" category can't rot.
• Type-aware lint replacing as casts with real narrowing in NavbarLocationMenu, RegionNotice, PageSection, and dompurify.test.ts (querySelector<HTMLAnchorElement>("a") over body.firstChild as HTMLAnchorElement) is a real improvement, not just rule appeasement.

No files changed — review only.
· branch bun-ts7