[Programcode]

Fourth PR from the over-engineering audit. Scoped to src/lib and src/hooks. Independent of #34, #35, #36, and #38: no file overlap, based on current main.

Removed

Five exports, 23 lines. Each was verified with plain string greps across the whole tracked tree, not just typecheck, because untyped access through a Zustand store would not fail typecheck:

• AIRequest and AIResponse in src/lib/ai/providers/types.ts. Speculative provider DTOs from before AI calls moved behind the Edge Functions, which now own the request and response shapes. The rest of the file stays: AIProvider, AIModel, ProviderConfig, and DEFAULT_PROVIDERS are all live. README.md:591 contains an unrelated inline interface AIRequest in an API example; it does not import this type.
• moveSourceToFolder in the session store. A one-line delegate to updateSource that nothing calls; LibraryPage.tsx:395 calls updateSource directly, including for the folder field.
• resetOnboarding in the onboarding store. Its only consumer is OnboardingTour.tsx:10, which destructures isTourRunning, hasCompletedOnboarding, setOnboardingComplete, startTour, and stopTour — never this action. There is no window.__stores-style exposure anywhere and no page.evaluate store access in e2e/, so there is no untyped call path either.
• IS_STAGING in src/lib/supabase/env.ts. See below.

On IS_STAGING and the staging suite

Worth addressing head on, since this PR deletes the app's only staging flag while not running the staging suite.

$ git grep -n "IS_STAGING" -- .
(zero matches)

Zero matches in the entire tracked tree, including e2e/staging/, scripts/, ios/, public/, nginx.conf, and vercel.json. There is no barrel that re-exports env.ts either.

The staging suite cannot be affected, and the reason is structural rather than a grep result: e2e/staging/auth.setup.ts and e2e/staging/authed-flows.spec.ts never import from src/. They construct their own client from @supabase/supabase-js and read the STAGING_SUPABASE_URL / STAGING_SUPABASE_ANON_KEY / STAGING_DEMO_CODE environment variables. IS_STAGING was a browser-bundle flag with no consumer on either side.

An earlier revision of this description said "nothing here is staging dependent", which was loose wording for a PR that deletes a symbol with STAGING in its name. The claim above is the precise one.

Rejected during this pass

Three findings from the audit that I tried and backed out. Each is a case where the shorter form is wrong, so I'd rather record why than leave them to be rediscovered:

1. Dropping the FileReader fallbacks in src/lib/source-file-import.ts (~30 lines, the largest cut in this slice). file.text() and file.arrayBuffer() exist in every browser this app targets, so the fallbacks look like dead compatibility code. They are not, and the empirical result is the justification: removing them fails 6 tests with TypeError: file.text is not a function. Mechanism, pinned to this lockfile rather than stated as a general jsdom fact — jsdom 20.0.3, the version installed here, implements neither:

   $ node -e "const {JSDOM}=require('jsdom'); const f=new (new JSDOM().window.File)(['x'],'a.txt');
              console.log(typeof f.text, typeof f.arrayBuffer)"
   undefined undefined
   

   Keeping the fallback is strictly less code than removing it and polyfilling the test setup.

2. Replacing createId()'s UUID fallback with crypto.randomUUID() in the sparring store. crypto.randomUUID is only defined in a secure context, so this would be undefined and throw when the dev server is reached over a plain-HTTP LAN address. Six lines is not worth that.

3. Removing BLOCKED_PROTOCOLS from src/lib/validation/url.ts. The ALLOWED_PROTOCOLS allowlist immediately below already rejects every scheme the blocklist names, so the check is redundant for the reject decision. It is still deliberate defense in depth at a trust boundary and emits a distinct [SECURITY] log line for dangerous schemes. Not worth trading for 6 lines.

Verification

check	result
npm run lint	pass
npm run typecheck	pass
npm test -- --coverage	95 files, 925 tests pass, coverage 90.50% stmts
npm run test:edge	121 pass
npm run build	pass
npm run test:e2e	125 pass, 1 unrelated flaky retry in search-quality

Not run: WebKit smoke, and the protected staging suite, whose independence from this diff is established above.

Summary by CodeRabbit

• Changes
  • Removed the ability to reset onboarding completion and tour progress.
  • Existing source-folder updates remain available through the standard source update flow.
• Internal Updates
  • Simplified internal onboarding, AI provider, session, and environment interfaces without changing supported provider configurations or environment behavior.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated
debate-navigator	Ready	Preview	Sep 6, 2026 8:45pm UTC

[coderabbitai]

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Free

Run ID: 25ba4854-ae3c-4076-a8a2-b41681e3bd7f

📥 Commits

Reviewing files that changed from the base of the PR and between c7d3437 and c876a1c.

📒 Files selected for processing (4)

• src/hooks/use-onboarding.ts
• src/lib/ai/providers/types.ts
• src/lib/stores/session-store.ts
• src/lib/supabase/env.ts

💤 Files with no reviewable changes (4)

• src/hooks/use-onboarding.ts
• src/lib/ai/providers/types.ts
• src/lib/stores/session-store.ts
• src/lib/supabase/env.ts

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

The PR removes obsolete onboarding and session actions, AI provider interfaces, and the Supabase staging environment export.

Changes

Public API cleanup

Layer / File(s)	Summary
Onboarding reset removal src/hooks/use-onboarding.ts	Removes resetOnboarding from the onboarding state contract and Zustand store.
Session folder action removal src/lib/stores/session-store.ts	Removes moveSourceToFolder from the session store contract and implementation.
Provider and environment export cleanup src/lib/ai/providers/types.ts, src/lib/supabase/env.ts	Removes AIRequest, AIResponse, and IS_STAGING exports.

Estimated code review effort: 1 (Trivial) | ~5 minutes

---

Note

🎁 Summarized by CodeRabbit Free

Your organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Essentials by visiting https://app.coderabbit.ai/settings/billing.

_{Comment @coderabbitai help to get the list of available commands.}

[Programcode]

Interim independent review while CodeRabbit is rate limited (Free plan allows 1 review/hour).

Reviewer: Claude Opus 4.5, run locally against git diff origin/main..origin/<branch> with a deletion-safety and claim-accuracy brief. Full output:

BLOCKERS: none

SHOULD-FIX: none

NITS:
1. The commit message says "925 unit tests" but the PR description table says "925 tests" under the coverage run. Trivial, but if these are meant to match exactly, confirm the number is consistent across both.

VERDICT: Approve. All four deletions are correctly justified—each symbol has no callers beyond its own declaration, the README `AIRequest` is an unrelated inline example, and the verification matrix (lint, typecheck, tests, build, e2e) confirms nothing broke.

CodeRabbit will still review this PR when its hourly window resets; this does not replace it.

[Programcode]

Independent review, second pass, model upgraded to Claude Opus 5 (the earlier comment on this PR used Opus 4.5). Run locally against git diff origin/main..origin/<branch> through cliproxyapi, briefed on deletion safety and claim accuracy.

Every should-fix finding below has been addressed in the current head. Full output:

BLOCKERS:
1. `IS_STAGING` removal vs. the verification claim. The PR states "Not run: WebKit smoke and the protected staging suite. Nothing here is rendering-engine or **staging dependent**." That is contradicted by the diff: `src/lib/supabase/env.ts:42` removes the app's only staging flag. The staging suite is precisely the path most likely to reference it (staging banner, env-gated assertion, `page.evaluate` on a window-exposed flag), and it is the one suite you did not run. Either run the staging suite or paste the repo-wide grep for `IS_STAGING` (including `e2e/`, `scripts/`, `vercel.json`/`nginx.conf` env wiring, and any `export * from './env'` barrel consumed outside `src`). Also fix the claim, which as written is false.

SHOULD-FIX:
1. `resetOnboarding` safety rests on typecheck, which cannot see untyped access. Zustand stores are commonly poked from e2e via `page.evaluate(() => window.__stores.onboarding.getState().resetOnboarding())` or from a dev/settings "restart tour" handler using bracket access. `npm run typecheck` passing does not cover either. Confirm with a plain string grep for `resetOnboarding` across `e2e/`, `public/`, `ios/`, and any `window.` store exposure. Note the store is `persist`-backed (`debate-trainer-onboarding`), so a "reset tour" affordance is a plausible caller.
2. Commit message scope is inaccurate in two ways: it says "four unreferenced **lib** exports" but there are five symbols removed, and one of them (`resetOnboarding`) is in `src/hooks/`, not `src/lib/`. The PR body has the correct scope; the commit is what lands in history.

NITS:
1. Rejected-item #1's evidence is stated more strongly than it holds: "jsdom's `File` implements neither" is version-dependent (`Blob.prototype.text`/`arrayBuffer` exist in current jsdom). The empirical part — "removing it fails 6 tests" — is the real justification; the mechanism claim should be softened or pinned to the jsdom version in the lockfile. No diff impact.

Correctly justified, no action needed:
- `AIRequest`/`AIResponse`: types-only, erased at build, no runtime or dynamic-import surface; the `README.md:591` inline `interface AIRequest` is explicitly caveated and is not an import. Blank-line handling in `types.ts` is clean, and `AIProvider`/`AIModel`/`ProviderConfig`/`DEFAULT_PROVIDERS` are untouched.
- `moveSourceToFolder`: a one-line delegate on a typed store interface; any caller in `src`/`e2e` would fail typecheck, and `updateSource` remains.
- `env.ts` leaves no orphan — `active` is still consumed by `ENVS[active]` twice.
- "23 lines" matches the diff exactly (2 + 16 + 4 + 1). Unit/edge/e2e counts are consistent between the commit message and PR body.
- No test was weakened; no test file appears in the diff.

VERDICT: request-changes — the PR asserts nothing staging-dependent while deleting the staging flag, and the staging suite was never run.

[Programcode]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.