This repository was archived by the owner on May 13, 2026. It is now read-only.
feat(ts): Phase 1.2 partial — strict-type 3 api/clients + 2 tests - #808
Merged
Conversation
Snapshots the 184 .js/.jsx files in src/server/static-react/ at migration start. CI fails when a new tracked .js/.jsx is added outside the allowlist (or scripts/, vite/eslint/tailwind/postcss/vitest configs) and when an allowlist entry is missing — converting a file forces a matching allowlist deletion, so the list shrinks monotonically and deletes itself when the migration is done. Wired into both the lint job in ci-tests.yml and the pre-commit hook so new debt is caught locally before push. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Removes @ts-nocheck from the two foundational API modules. Touches schemaClient.ts only to drop a dead import that broke when the unused SchemaApiClient interface (whose SchemaData referent never existed) was removed. - types.ts: move ErrorInterceptor here from client.ts so ApiClientInstance can reference it; widen RequestConfig.body to unknown to match what ApiClient.serializeBody actually accepts; delete dead SchemaApiClient interface (no implementer; referenced undefined SchemaData). - client.ts: import ErrorInterceptor from types.ts instead of redeclaring; drop unused NetworkError/TimeoutError imports; guard the Map iterator result before delete (firstKey can be undefined); cast the queue.get result to Promise<T> with a comment on why the map is keyed unknown. Phase 1 step 1 of the gradual TS migration. 14 @ts-nocheck files remain. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
#804 converted it to .ts. The lint script catches this drift, but the fix is mechanical — no review needed beyond confirming the file is gone. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Removes @ts-nocheck from llmQueryClient, systemClient, indexingClient and the schemaClient/activateExemem test files. Bug surfaced (llmQueryClient): the per-instance retry override was being silently ignored. ApiClientConfig calls the field `retryAttempts`, but the constructor was passed `retries: API_RETRIES.LIMITED` which is not in the config interface — it just fell through to API_RETRY_ATTEMPTS defaults. Fixed by renaming to `retryAttempts`. LLM calls now actually use the limited retry budget the original author intended. Cleanup (systemClient): two `SyncStatusResponse` interfaces with the same name were silently merged under @ts-nocheck — one with a narrow state union, one with `string | null` and an `encryption_active` field. Merged into a single interface with every field optional so existing consumers (BackupSettingsPanel.tsx's locally-typed shape, the untyped SyncStatusIndicator.jsx) keep working. Hardening (indexingClient): getIndexingStatus now throws if the response has no data instead of silently returning undefined typed as the success shape. Matches the project's "no silent failures" rule. mutationClient.ts NOT included — it imports from `@generated/generated` which lives in `bindings/` (gitignored, written by ts-rs during cargo test --features ts-bindings). Frontend CI never runs cargo, so the import path doesn't resolve in either the local or CI typecheck. All files that use @generated today are @ts-nocheck'd, which masks this. Unblocking mutationClient (and later phases that touch @generated: types/schema.ts, utils/filterUtils.ts, hooks/useQueryBuilder.ts) needs a separate PR that wires binding generation into CI before frontend typecheck runs. Filed as a follow-up. 11 @ts-nocheck files remain. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CI's TypeScript flagged the @ts-expect-error on the global.fetch assignment as unused (TS2578). Local typecheck passed because of node_modules drift; the underlying issue is that newer vitest types make vi.fn() assignable enough to typeof global.fetch that the directive is unnecessary. Replaced with `as unknown as typeof globalThis.fetch` — explicit cast, no directive needed, works regardless of vitest type version. Also switched `global` → `globalThis` so the file isn't relying on the Node ambient global. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
1 participant
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
Phase 1.2 of the TypeScript migration. Removes `@ts-nocheck` from 5 files in `src/api/clients/`:
llmQueryClient.ts— surfaced a real bug (see below)systemClient.ts— merged duplicateSyncStatusResponseinterfacesindexingClient.ts— added explicit error throw on missing response data__tests__/activateExemem.test.ts__tests__/schemaClient.test.ts11
@ts-nocheckfiles remain.Stacked on #807
Base =
claude/stoic-chebyshev-894590(Phase 0 + Phase 1.1). Will retarget tomainonce #807 lands.Bug surfaced: llmQueryClient retries were silently ignored
The LLM client was constructed with:
ApiClientConfighas noretriesfield — the correct name isretryAttempts. With@ts-nocheckmasking the type error, the override silently fell through to the default (API_RETRY_ATTEMPTS = 3). LLM calls (which the author wanted to retry only once due to high latency / cost) were retrying up to 3 times. Fixed by renaming.Cleanup: duplicate
SyncStatusResponseinterfacessystemClient.tshad two declarations ofSyncStatusResponsewith the same name but different shapes — one with a narrow state union, one withstring | nullplusencryption_active. TS would have flagged this immediately, but@ts-nocheckmasked it. Merged into a single interface with all fields optional, so both consumers (`BackupSettingsPanel.tsx`'s locally-typed shape, the untyped `SyncStatusIndicator.jsx`) keep working without changes.Hardening: indexingClient now throws on missing data
`getIndexingStatus` previously returned `response.data` typed as `IndexingStatus` even when `data` was undefined. Added an explicit throw — matches the project's "no silent failures" rule from CLAUDE.md.
Why mutationClient.ts is NOT in this PR
It imports from `@generated/generated`, which lives in `bindings/` — a gitignored directory written by `ts-rs` during `cargo test --features ts-bindings`. Frontend CI never runs `cargo`, so this path doesn't resolve in either local or CI typecheck. Today, every consumer of `@generated/*` carries `@ts-nocheck`, which masks the missing module. Removing the directive reveals the gap.
Files blocked on this:
Unblocking PR (separate, before further migration phases): wire `cargo test -p fold_db_node --features ts-bindings` (or the equivalent ts-rs export step) into the frontend-tests CI job so `bindings/` exists by the time `tsc --noEmit` runs. Then mutationClient can be typed in a follow-up.
Test plan
🤖 Generated with Claude Code