Skip to content

Audit re-triage (all 21 closed), line-level H2 sanitizer fix, fast-check property suite#338

Merged
BigSimmo merged 8 commits into
mainfrom
claude/audit-sweep-property-tests
Jul 7, 2026
Merged

Audit re-triage (all 21 closed), line-level H2 sanitizer fix, fast-check property suite#338
BigSimmo merged 8 commits into
mainfrom
claude/audit-sweep-property-tests

Conversation

@BigSimmo

@BigSimmo BigSimmo commented Jul 6, 2026

Copy link
Copy Markdown
Owner

Summary

  • Phase 1 — audit re-triage: every finding from docs/audit/repo-audit-2026-07-01.md (H1–H4, M1–M17) was verified against current main, code-level, not comment-level. All 21 are closed (table below), so the planned Phase 2 fix list was a no-op.
  • Phase 3 — property-based tests: added fast-check (dev dependency) and three property suites wired into vitest, pinning the text-processing core's clinical invariants over generated inputs.
  • Bonus fix the property suite immediately caught: a live line-level H2 residue in stripLowYieldLines (src/lib/source-text-sanitizer.ts) — extraction glues document-control markers ("Document owner:", "review date") onto body text, and several callers compact excerpts to a single line before the filter runs, so a line carrying threshold values but no clinical keyword was deleted wholesale. Shrunken counterexample: "The Glasgow Coma Scale ranges from 3 to 15 with 1 or below indicating severe head injury. Document owner: Pharmacy Department." sanitized to an empty string. The line-level heuristics now get the same clinicalThresholdSignalPattern rescue the fragment-level heuristics received in the original H2 fix.

Triage table (all 21 findings vs current main)

ID Status Evidence on current main
H1 Closed sourceTextForResult includes retrieval_synopsis, image text, and table-fact metadata (answer-verification.ts:112–132, incl. the R4 completion)
H2 Closed (fragment level) · line-level residue fixed in this PR clinicalThresholdSignalPattern rescue in clinicalProseUsefulness (source-text-sanitizer.ts); stripLowYieldLines lacked the same rescue → commit 1 here
H3 Closed — superseded by PR #118, no change made The penalties the audit says are discarded (outdated −0.24, review_due −0.12, unverified −0.08, poor-extraction −0.12) no longer exist: #118 removed them entirely, measured (golden doc-recall@5 1.0→0.76 with them), with a "do not reintroduce" comment at retrieval-selection.ts:329–336. The residual Math.max floor only affects intent-based boosts, and un-flooring it was already tried and withdrawn (audit R1: penalties compounded across the 2–3 selection passes). Re-opening it requires the live golden eval, which this environment cannot run — deliberately untouched.
H4 Closed clinicalTableToTextRows routes through normalizeAccessibleTable (conservativeClinical) and emits the caveat line (ward-output.ts:604–636); now also property-tested
M1 Closed deriveConfidence scoped to cited chunks (rag.ts:744–753)
M2 Closed word-boundary signal matching in containsAny (clinical-search.ts:605–615)
M3 Closed explicitDoseTerms survive escalation cancellation (clinical-search.ts:796–813)
M4 Closed structuralTable guard before the bare yes/no flowchart branch (document-index-units.ts:290–296)
M5 Closed reference fallbacks gate on confirmedCandidates (document-organization.ts:1668–1689)
M6 Closed title/file_name/source_path in evidenceText, bracket-stripped (document-organization.ts:1566–1588)
M7 Closed BMJ attribution requires the \bbmj\b token (document-organization.ts:270–275)
M8 Closed markdown header row never re-emitted as data (ward-output.ts:612–622); property-tested
M9 Closed DELETE blocks pending + processing, plus the PR #278 TOCTOU re-check (documents/[id]/route.ts:504–558)
M10 Closed searchRequestSeqRef request-token guard incl. progress messages (ClinicalDashboard.tsx:2646+)
M11 Closed embeddings computed before deleting old memory rows (deep-memory.ts:646–650)
M12 Closed ph2 192-bit perceptual hash, version-bumped (image-filtering.ts:343–385)
M13 Closed (repo-side) migration 20260702000000_commit_generation_preserve_legacy_artifacts.sql in-tree; live-apply verification (npm run check:m13-migration) needs live keys unavailable here
M14 Closed page-noise pattern anchored to the whole line (chunking.ts:20–25)
M15 Closed HMAC-SHA256 keyed hash when RAG_QUERY_HASH_SECRET set (query-privacy.ts)
M16 Closed ragged rows padded/merged via normalizeAccessibleTable (ward-output.ts); property-tested
M17 Closed strict forward-progress guard in chunkTextBySentence (chunking.ts:363–369); property-tested incl. overlap >= chunkSize

Property suite (Phase 3)

  • tests/property-numeric-token-preservation.test.ts — no unit-bearing numeric token (doses, ranges, percentages, comparatives) ever disappears through clinicalProseUsefulness, sourceTextForDisplay, sourceTextForModel, or sourceTextForVerbatimQuote, across generated clinical prose with injected provenance/control/banner noise. This suite found the line-level H2 residue.
  • tests/property-chunking.test.tschunkTextWithOverlap terminates for any size/overlap combination including overlap >= chunkSize (the M17 region), loses no characters (order-preserving subsequence check tolerant of overlap duplication), and invents nothing (every chunk is a contiguous run of the input).
  • tests/property-accessible-table.test.tsnormalizeAccessibleTable always yields a rectangular grid, never invents or loses numeric values, preserves row counts for first-cell-anchored rows, flags ambiguous clinical tables low-confidence with the raw grid preserved 1:1, and the low-confidence caveat survives both formatAnswerForClipboard and formatWardNote (H4), with a non-vacuity canary.
  • Stability: suites re-run 5× with fresh random seeds, all green.

Side observation (no change made here): removePageNoise deliberately drops whole lines of ≤ 2 chars as extraction debris, which would delete a wrapped unit-only line ("12.5\nmg" → "mg" line lost). The property generators respect that documented contract. A spun-off session is fixing this on claude/chunking-unit-lines (rejoinWrappedDoseUnits); commit 8bd318670 on this branch (authored by that session) updates the generator comment to forward-reference it — the function lands via that branch, not this one.

Verification

  • npm run verify:cheap — see eval/verification status below
  • npm run verify:ui — N/A (no UI/routing/styling change)
  • npm run verify:release — N/A
  • npm run format:check — clean on all changed files
  • npm run eval:retrieval:qualityN/A for this branch: no retrieval/selection/ranking/chunking behavior changed (H3 deliberately untouched; chunking changes are tests-only)
  • npm run eval:rag -- --limit 15 + npm run eval:quality -- --rag-onlyREQUIRED before merge, could not run: this machine has no live Supabase/OpenAI keys (npm run check:supabase-project reports env unset; no .env.local exists). The sanitizer fix is answer-path post-processing, so per the standing gate these must be run before merging. Debt recorded in docs/process-hardening.md ("Audit re-triage & property-test suite (2026-07-06)"). The change is strictly keep-more-text (a line is rescued if it carries threshold values; nothing new is dropped), so grounded-supported should not drop — but the gate still needs to be run.
  • npm run check:production-readiness — not run (no env/privacy/governance behavior change; sanitizer change is covered by the eval gate above)
  • npm run check:deployment-readiness — N/A

Test/verify status detail:

  • Final verify:cheap on the merged tree (post origin/main merge): exit 0 — 132 files / 1194 tests, all passing, plus check:runtime, sitemap:check, lint, and typecheck.
  • Property suites additionally re-run 5× with fresh random seeds (all green) and once more alongside the adjacent unit suites (ward-output, chunking, accessible-table-normalization, source-text-sanitizer): 97/97.
  • One earlier pre-merge run saw tests/universal-search.test.ts (untouched by this branch) hit its 15s per-test timeout under parallel load; it passes 8/8 in isolation and in the final gate — load-induced flake, noting for visibility.

Clinical Governance Preflight

  • Source-backed claims still require linked source verification before clinical use (unchanged; sanitizer change only preserves more source text)
  • No patient-identifiable document workflow was introduced or expanded
  • Supabase target remains Clinical KB Database (sjrfecxgysukkwxsowpy) — no Supabase config touched
  • Service-role keys and private document access remain server-only
  • Demo/synthetic content remains clearly separated from real clinical sources
  • Source metadata, review status, and outdated/unknown-source behavior remain conservative (the fix leans further conservative: threshold-bearing lines are kept, pure control lines still dropped — pinned by test)
  • Deployment classification/TGA SaMD impact: no decision-support behavior change beyond preserving source text that was being wrongly deleted

Notes

  • Commits are independently revertible: (1) sanitizer fix + deterministic regression tests, (2) fast-check + property suites, (3) docs. Reverting (1) alone will make the property suite fail — that is the regression guard working.
  • origin/main (56 commits, including the tools-catalog and cross-mode-links merges) was merged into this branch before opening the PR; the only conflict was the appended docs/process-hardening.md section (both entries kept). verify:cheap was re-run on the merged tree.
  • The triage was performed against pre-merge main (935e9d6ab). Of the 56 new main commits, the only audited file touched is ClinicalDashboard.tsx, and its M10 request-token guard (searchRequestSeqRef) is untouched and still present post-merge (re-verified). All other audited functions are unchanged by the new commits.
  • Live mode unavailable in the authoring environment: npm run check:supabase-projectNEXT_PUBLIC_SUPABASE_URL/SUPABASE_PROJECT_REF/SUPABASE_PROJECT_NAME all unset, and no .env.local exists on this machine. This is why the eval gates above are recorded as pre-merge debt rather than run.

🤖 Generated with Claude Code

BigSimmo and others added 5 commits July 6, 2026 22:01
…l markers

H2 line-level residue, found by the new fast-check property suite: extraction
glues control markers ('Document owner:', 'review date') onto body text, and
stripInternalImageDataBlocks compacts excerpts to a single line before
stripLowYieldLines runs — so the control-line filter deleted whole clinical
lines when they carried threshold values but no clinical keyword. Shrunken
counterexample: 'The Glasgow Coma Scale ranges from 3 to 15 with 1 or below
indicating severe head injury. Document owner: Pharmacy Department.'
sanitized to an empty string.

The line-level heuristics now get the same clinicalThresholdSignalPattern
rescue the fragment-level heuristics received in the original H2 fix: a line
carrying unit-bearing figures, ranges, or comparatives is never dropped by the
control-line/source-marker rules. Pure control lines without clinical values
are still removed (pinned by test).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Adds fast-check (dev dependency) and three property-test files wired into the
existing vitest suite:

- property-numeric-token-preservation: no unit-bearing numeric token (doses,
  ranges, percentages, comparatives) ever disappears through
  clinicalProseUsefulness, sourceTextForDisplay, sourceTextForModel, or
  sourceTextForVerbatimQuote, across generated clinical prose with injected
  provenance/control/banner noise. This suite caught the line-level H2
  residue fixed in the previous commit.
- property-chunking: chunkTextWithOverlap terminates for any size/overlap
  combination including overlap >= chunkSize (audit M17 region), loses no
  characters (order-preserving subsequence check tolerant of overlap
  duplication), and invents nothing (every chunk is a contiguous run of the
  input). Generators respect removePageNoise's documented line contracts so
  the assertions target the chunker, not the noise filter.
- property-accessible-table: normalizeAccessibleTable always yields a
  rectangular grid, never invents or loses numeric values, preserves row
  counts for first-cell-anchored rows, flags ambiguous clinical tables
  low-confidence with the raw grid preserved 1:1, and the low-confidence
  caveat survives both formatAnswerForClipboard and formatWardNote (audit
  H4/M8/M16 invariants), with a non-vacuity canary.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
All 21 findings (H1-H4, M1-M17) from the 2026-07-01 audit verified closed on
current main; H3 closed by PR #118 supersession. Documents the eval debt for
the sanitizer fix (eval:quality --rag-only needs live keys) per the standing
gates.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ract comment

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…operty-tests

# Conflicts:
#	docs/process-hardening.md
@supabase

supabase Bot commented Jul 6, 2026

Copy link
Copy Markdown

This pull request has been ignored for the connected project sjrfecxgysukkwxsowpy because there are no changes detected in supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.


Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

@BigSimmo
BigSimmo marked this pull request as ready for review July 6, 2026 18:18
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@BigSimmo
BigSimmo enabled auto-merge July 6, 2026 18:18
@BigSimmo

BigSimmo commented Jul 6, 2026

Copy link
Copy Markdown
Owner Author

CI status note: all three checks (verify, Gitleaks, ui-smoke) failed at job start, before any step ran — GitHub reports "The job was not started because recent account payments have failed or your spending limit needs to be increased" (Actions billing). Not a code failure. Local verify:cheap passed exit 0 on this exact commit (f83304590): 132 files / 1194 tests, plus lint, typecheck, check:runtime, sitemap:check, and format:check. Re-run the checks once billing is resolved.

🤖 Generated with Claude Code

cursoragent and others added 3 commits July 7, 2026 03:19
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
…operty-tests

# Conflicts:
#	docs/process-hardening.md
@BigSimmo
BigSimmo merged commit ed23069 into main Jul 7, 2026
5 checks passed
@BigSimmo
BigSimmo deleted the claude/audit-sweep-property-tests branch July 8, 2026 16:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants