Fix type-honesty gaps from PR #131 typed-client migration - #174
Merged
Conversation
- planDocumentName: select the metadata column so the stored smart_title_group_key actually participates in duplicate-title grouping (the typed select silently dropped it, leaving the metadata branch dead on the DB-backed path) - rag.ts: derive RagQueryInsert from the generated rag_queries Insert type instead of a hand-written interface, removing the whole-row cast and the source_chunk_ids null lie (column is NOT NULL) - rag.ts: validate document_images.bbox jsonb as a [x0,y0,x1,y1] tuple at the read boundary instead of blind-casting Json Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 45a0d3c874
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Drops the branch-local chunkImageBbox in favour of the shared helper that landed on main with the same semantics. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
BigSimmo
enabled auto-merge (squash)
July 2, 2026 11:51
Whitespace/wrapping only; keeps npm run format:check green after merging origin/main. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
…oke) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- document-naming: also match a renamed document by its current title, not only the stored smart_title_group_key (renames update title but preserve metadata, so the stored key can be stale and miss a duplicate) - image-filtering: normalizeImageBbox rejects non-numeric coordinates (null/""/false) instead of Number()-coercing them to a bogus 0 - tests: cover stale-group-key rename matching and non-numeric bbox entries Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…claude/pr131-review-fixes
BigSimmo
added a commit
that referenced
this pull request
Jul 2, 2026
…yInsert - Run prettier --write to clear the format:check gate (20 files: mockup components, governance scripts, rag.ts, ui-smoke.spec.ts, etc.). No trap files touched (worker/main.ts and database.types.ts stay .prettierignore'd). - Fix a latent typecheck error the format gate was masking: the main-merge kept logRagQuery(row: Record<string, unknown>) while #174 tightened insertRagQuery to require RagQueryInsert (mandatory query). Align the signature to main's logRagQuery(row: RagQueryInsert). Local: format:check, typecheck, lint (0 errors), and unit tests (863) all green. Co-Authored-By: Claude Fable 5 <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 join this conversation on GitHub.
Already have an account?
Sign in to comment
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
Post-merge review of #131 (typed Supabase client / generated DB types). Three gaps patched, plus a broken verification gate restored:
planDocumentNamenever fetchedmetadata, so thesmart_title_group_keywritten by the upload route and import script for duplicate-title grouping was silently ignored on the DB-backed path. The select now includesmetadata.RagQueryInsertdiverged from the real schema: it allowedsource_chunk_ids: null(column is NOT NULL) and the whole-rowascast at the insert would hide future column mismatches. It is now derived from the generatedrag_queriesInsert type; the only remaining cast is the narrow one on the merged privacy metadata object.document_images.bboxwas blind-cast from untyped jsonb to the new[x0,y0,x1,y1]tuple type. It is now validated at the read boundary (chunkImageBbox) and falls back tonullfor malformed values.npm run format:checkfailed on main: the generateddatabase.types.ts(from Tighten TypeScript: generated DB types, typed Supabase client, strict env #131) is now prettier-ignored (regeneration would keep fighting the formatter), anddocument-viewer-lazy.tsx(from perf: SSR-safe lazy loading, loading skeletons, and SourceImage memo #147) is formatted.Reviewed and cleared without changes:
extraction_modeunion narrowing (verified against live data — only the three union values exist),indexingVersionnarrowing (only assigned from theragDeepMemoryVersionconstant),requireServerEnvtyped return, typed admin client, generated types freshness (spot-checked against the six newest migration RPCs),noImplicitReturns.Verification
npm run verify:cheap— check:runtime, lint, typecheck, vitest: 98 files / 821 tests passed (2 skipped, pre-existing)npm run verify:ui— N/A: no UI, routing, or styling behavior changed (viewer file change is prettier whitespace only)npm run verify:release— not run; not a release/handoff claimnpm run format:check— passes after this PR (failed on main)npm run check:production-readiness—:cimode: READY, no blocking failures (env-less scratch worktree, missing-env checks downgraded as designed)npm run check:deployment-readiness— N/A: no deployment startup/hosting changeClinical Governance Preflight
queryTextForStorage/queryPrivacyMetadata) unchangedClinical KB Database(sjrfecxgysukkwxsowpy)nullinstead of a lying castNotes
scripts/enrich-documents.ts→image-filtering.ts) is being handled in a separate session.🤖 Generated with Claude Code