Skip to content

Cover clinical calculator scoring and the batch image signed-URL route - #1383

Merged
BigSimmo merged 6 commits into
mainfrom
claude/test-coverage-analysis-2vcd8a
Jul 29, 2026
Merged

Cover clinical calculator scoring and the batch image signed-URL route#1383
BigSimmo merged 6 commits into
mainfrom
claude/test-coverage-analysis-2vcd8a

Conversation

@BigSimmo

Copy link
Copy Markdown
Owner

Summary

Adds executing test coverage to the two highest-risk surfaces in the repo that had none, and re-ratchets the broad coverage floor that had drifted well below measured coverage. Test, config and docs only — no production source changed.

This came out of a measured coverage analysis (npm run test:coverage, baseline 51.05% statements / 52.48% lines overall). The clinical core is already strong — src/lib/rag/ sits at 90.8% lines — so the gaps are all outside the surface this repo has historically defended.

1. Clinical calculator scoringsrc/components/calculators/calculator-ui.tsx was at 0% executed lines. Its exported pure functions carry the scoring and interpretation for eight validated instruments (PHQ-9, GAD-7, K10, MDQ, CAGE, AUDIT-C, SAD PERSONS, Y-BOCS), and the only calculator test in the repo asserted mockup import boundaries. A wrong severity band renders as an ordinary result, so it needs a regression floor. tests/calculator-scoring.test.ts (60 tests) covers band-table integrity for all eight instruments — contiguous, spanning the declared range, exactly one band per reachable score, and the range actually reachable from the items — plus PHQ-9/GAD-7 boundary scores either side of every cut point, itemScore edge cases, band suppression for scales that cannot read zero (K10 floors at 10; CAGE is checkbox-only), the MDQ three-criterion rule, and the result summary text.

No live defect was found — the band tables are correct today. Nothing was holding them there.

2. Batch image signed URLssrc/app/api/images/signed-urls/route.ts was at 0% executed lines with no test references anywhere. It mints up to 100 signed URLs per call and carries its own owner-scope and committed-generation implementation, parallel to /api/images/[id]/signed-url, which already had five cases in tests/private-access-routes.test.ts. check:owner-scope-api passes it statically because withOwnerReadScope appears in the handler, but it cannot cover the real protection: document_images has no owner_id column, so the only thing excluding another owner's image is the documentMap join filter. Nine cases now pin that filter, including the batch-specific case where one unowned id rides along with an owned one.

3. Coverage floor — the broad threshold group measured 63.99 / 55.29 / 57.6 / 66.19 against floors of 48 / 38 / 43 / 50: 14–17pp of slack on every dimension, so a change could delete a large amount of coverage and still pass. Each floor now sits ~2pp under measured — enough headroom for a PR that ships an uncovered surface, not enough to hide a regression.

Both test additions were mutation-checked rather than assumed:

Mutation Result
PHQ-9 Minimal band max: 45 (overlaps Mild) 3 tests fail, including scores 5 as Mild
MDQ impairment threshold >= 2 (moderate) → >= 1 (minor) 1 test fails
Batch route documentMap miss returns true instead of false 2 cross-owner tests fail

The third mutation is the important one: before this change it passed the entire 4,271-test suite silently.

Efficiency. Additions were kept deliberately in the cheapest tiers. Measured on this suite, 46% of unit-suite worker time is test execution and 54% is import/environment/transform overhead, so a new test file costs ~0.37s before it asserts anything while an assertion in an existing file costs ~0.03s. The calculator work is therefore one new file (60 tests in 514ms, of which 27ms is assertions), and the signed-URL work was appended to private-access-routes.test.ts to reuse its harness rather than starting a new file. Nothing was added to the Playwright gate, which runs ~231 production journeys serially against a 45-minute CI budget. Net: +69 tests, ~1s.

Two follow-ups from the same analysis are recorded in docs/outstanding-issues.md (#098 worker/edge modules verified only by source-text assertions; #099 component state matrices at 38.2% lines / 22.8% branch) rather than attempted here.

Verification

  • npm run verify:pr-local — exit 0. Test Files 423 passed (423), Tests 4340 passed | 4 skipped (4344); offline RAG fixture validation passed (36 golden cases, 21 suites); build skipped (no build-affecting changes detected).
  • npm run verify:cheap — exit 0. Test Files 423 passed (423), Tests 4340 passed | 4 skipped (4344).
  • npm run test:coverage — exit 0, no threshold errors against the new floors. src/app/api/images/signed-urls/route.ts 0% → 80.9% lines / 100% functions; src/components/calculators/calculator-ui.tsx 0% → 40.9% lines, with all exported scoring logic executed.
  • Mutation checks on both new suites (table above).
  • UI verification not run: this diff contains no UI, routing, styling, or browser-behaviour change — classifyPullRequestFiles reports ui: false.
  • Retrieval/answer evals not run: no retrieval, ranking, selection, chunking, scoring, or answer-generation behaviour changed — classifyPullRequestFiles reports ragRanking: false. Offline fixture validation ran as part of verify:pr-local.
  • npm run check:production-readiness not run: no clinical workflow, privacy, environment, Supabase, source-governance, or deployment behaviour changed.

Risk and rollout

  • Risk: low — no production source file is modified — the diff is one new test file, added cases plus a storage-mock method in an existing test file, a coverage-threshold change in vitest.config.mts, and two ledger rows. The only behavioural effect on CI is that the required coverage job now fails when the broad group drops more than ~2pp below current coverage, which is the intended effect. classifyPullRequestFiles reports operationalRisk: true solely because vitest.config.mts matches the config pattern.
  • Rollback: revert this PR. Reverting restores the previous coverage floors (48/38/43/50) and removes the new tests; nothing else depends on them, no data or schema is touched, and no deploy step is involved. If only the floor proves too tight — for example a PR that legitimately ships a large uncovered surface — lower the four values in vitest.config.mts without touching the tests, and re-measure with npm run test:coverage.
  • Provider or production effects: None. No provider-backed gate was run and no deploy step is involved.

Clinical Governance Preflight

Not applicable to this diff. classifyPullRequestFiles reports clinicalRisk: false — the changed paths are tests/**, vitest.config.mts and docs/outstanding-issues.md, none of which match the clinical-risk patterns in scripts/pr-policy.mjs. No ingestion, answer generation, search/ranking, source rendering, document access, privacy, or clinical output behaviour changed; the calculator tests observe existing scoring logic without altering it.

Notes

The calculator suite covers the exported scoring functions. Three module-private helpers — seedCheckboxDefaults, toggleCheckboxAnswer, selectOptionAnswer — remain uncovered because they are reachable only through React event handlers, which is the jsdom tier rather than this one. seedCheckboxDefaults is what makes an all-negative CAGE / SAD PERSONS screen read as a valid 0 rather than incomplete, so it is named explicitly as a first target in #099 rather than left as an unlabelled gap.


Generated by Claude Code

claude added 2 commits July 29, 2026 04:39
…atrix coverage (#99)

Both recorded from the 2026-07-29 test-coverage analysis with measured
evidence so the follow-ups survive the session context.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012YRCXgX4AWZ579bKN6sk6b
…L route

Adds executing coverage to the two highest-risk surfaces that had none, and
re-ratchets the broad coverage floor that had drifted well below measured.

Clinical calculator scoring (src/components/calculators/calculator-ui.tsx,
previously 0% executed lines). The exported pure functions carry the scoring
and interpretation for eight validated instruments — PHQ-9, GAD-7, K10, MDQ,
CAGE, AUDIT-C, SAD PERSONS, Y-BOCS — and the only calculator test in the repo
asserted mockup import boundaries. A wrong severity band renders as an ordinary
result, so it needs a floor. tests/calculator-scoring.test.ts covers band-table
integrity for all eight instruments (contiguous, spanning the declared range,
exactly one band per reachable score, range actually reachable from the items),
PHQ-9/GAD-7 boundary scores either side of every cut point, itemScore edge
cases, band suppression for scales that cannot read zero (K10, CAGE), the MDQ
three-criterion rule, and the result summary text. No live defect was found —
the band tables are correct today; nothing was holding them there.

Batch image signed URLs (src/app/api/images/signed-urls/route.ts, previously 0%
executed lines, no test references). The route mints up to 100 signed URLs per
call and carries its own owner-scope and committed-generation implementation,
parallel to /api/images/[id]/signed-url which already had five cases.
check:owner-scope-api cannot cover the real protection because document_images
has no owner_id column: the only thing excluding another owner's image is the
documentMap join filter in the handler. Nine cases now pin that filter,
including the batch-specific case where one unowned id rides along with an
owned one. Added to tests/private-access-routes.test.ts rather than a new file
so it reuses that harness; createSignedUrls was added to the storage mock.

Both additions were mutation-checked. Overlapping a PHQ-9 band fails three
tests including "scores 5 as Mild"; loosening the MDQ impairment threshold from
moderate to minor fails one; making the batch route allow a documentMap miss
fails the two cross-owner cases. Before this change that last mutation passed
the entire suite silently.

Coverage floor: the broad group measured 63.99/55.29/57.6/66.19 against floors
of 48/38/43/50 — 14-17pp of slack, so a change could delete a large amount of
coverage and still pass. Each floor now sits ~2pp under measured.

Kept deliberately in the cheap tiers: the unit suite is 46% test execution and
54% import/environment overhead, so a new file costs ~0.37s before it asserts
anything while an assertion in an existing file costs ~0.03s. The calculator
file runs 60 tests in 514ms (27ms of that is assertions). Nothing was added to
the Playwright gate, which runs ~231 journeys serially against a 45-minute
budget.

Verified: npm run verify:cheap (exit 0; 423 files, 4340 passed | 4 skipped) and
npm run test:coverage (exit 0, no threshold errors).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012YRCXgX4AWZ579bKN6sk6b
@supabase

supabase Bot commented Jul 29, 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 ↗︎.

@coderabbitai

coderabbitai Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 53 minutes

Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 1457079f-3f89-4174-bd70-6fa838e69253

📥 Commits

Reviewing files that changed from the base of the PR and between 2134113 and bebc6c0.

📒 Files selected for processing (5)
  • docs/branch-review-ledger.md
  • docs/outstanding-issues.md
  • tests/calculator-scoring.test.ts
  • tests/private-access-routes.test.ts
  • vitest.config.mts

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

@BigSimmo

Copy link
Copy Markdown
Owner Author

@cursoragent Inspect the PR and repository state, identify the root cause of every failing CI check, unresolved review comment, merge conflict, and related defect. Apply the smallest robust fix using existing project patterns and best practices, without unrelated changes. Run Bugbot and all relevant lint, type-check, test, build, and CI-equivalent commands, fix any issues found, then resolve only the review comments that are fully addressed. Summarise the root causes, changes made, validation results, and any remaining blockers.

@cursor

cursor Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Taking a look!

Open in Web Open in Cursor 

@BigSimmo
BigSimmo marked this pull request as ready for review July 29, 2026 11:24
@BigSimmo

Copy link
Copy Markdown
Owner Author

@cursoragent Inspect the PR and repository state, identify the root cause of every failing CI check, unresolved review comment, merge conflict, and related defect. Apply the smallest robust fix using existing project patterns and best practices, without unrelated changes. Run Bugbot and all relevant lint, type-check, test, build, and CI-equivalent commands, fix any issues found, then resolve only the review comments that are fully addressed. Summarise the root causes, changes made, validation results, and any remaining blockers.

@BigSimmo
BigSimmo enabled auto-merge (squash) July 29, 2026 11:24
@cursor

cursor Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

PR #1383 is merged (f4a3f500, 2026-07-29 11:36:24Z). No remaining code defects to fix.

Root causes (from this babysit)

Issue Finding
Merge conflict Was real: docs/outstanding-issues.md ID clash (#098/#099 vs main’s latency #098#105) — fixed earlier by renumbering to #106/#107
Failing CI None on tip bebc6c02
Review threads None
Bugbot No cursor[bot] findings

Hosted CI on merged tip bebc6c02

All required checks PASS, including:

  • PR required
  • Unit coverage (4m41s)
  • Static PR checks, Safety, Migration replay
  • CircleCI verify
  • Semgrep / Gitleaks / PR policy

Changes this pass

  • Confirmed CI green (no product code changes needed)
  • Synced one more clean main commit after merge landed
  • Appended babysit closeout to docs/branch-review-ledger.md on the feature branch

Remaining

  • None for this PR — it has already landed via squash auto-merge
  • Post-merge ledger closeout commit (a150eefb) is only on claude/test-coverage-analysis-2vcd8a, not yet on main (optional hygiene if you want that row on main)
Open in Web Open in Cursor 

cursoragent and others added 4 commits July 29, 2026 11:26
Main already claimed #98#105 for the latency audit. Renumber this
branch's coverage follow-ups to #106/#107 and bump next-id to 108 so
the ledger stays monotonic with no duplicate IDs.

Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Ledger row for HEAD 0922d7f: conflict cleared by renumbering
coverage follow-ups to #106/#107; verify:cheap and coverage green.

Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
…analysis-2vcd8a

Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>

Copy link
Copy Markdown
Owner Author

The three red PR required results on this PR are concurrency cancellations, not defects — there is no failing check to fix here.

Four commits landed in quick succession (0922d7f6f476b5b72bc20bebc6c0). The workflow uses cancel-in-progress: true, so each push cancelled the run before it, and the aggregate calls require_success, which treats cancelled as a failure. Job logs for all three are identical:

CHANGES_RESULT: cancelled    STATIC_RESULT: cancelled
SAFETY_RESULT: cancelled     COVERAGE_RESULT: cancelled
BUILD_RESULT: cancelled      UI_RESULT: cancelled
##[error]changes result was cancelled

This is the already-tracked #095 in docs/outstanding-issues.md ("PR required reports failure for concurrency-cancelled jobs"), not a regression from this branch.

The run for the current head bebc6c0 (30447852418) is proceeding with no cancellations — Change scope, Static PR checks, Safety and config, PR policy, Semgrep, Semgrep ingestion gate, Gitleaks and GitGuardian are all green, with Unit coverage and Migration replay still running.

I also verified the one thing the main merge could genuinely have broken. That merge brought in 84 files / ~2,100 insertions, which could have dropped the group below the re-ratcheted coverage floor in vitest.config.mts. It does not: npm run test:coverage at bebc6c0 exits 0 with no threshold errors — Test Files 424 passed (424), Tests 4371 passed | 4 skipped (4375).

No push is needed, and pushing would make this worse — another commit would cancel the in-flight run and produce a fourth identical false failure. The correct action is to let 30447852418 finish.


Generated by Claude Code

@BigSimmo
BigSimmo merged commit f4a3f50 into main Jul 29, 2026
19 checks passed
@BigSimmo
BigSimmo deleted the claude/test-coverage-analysis-2vcd8a branch July 29, 2026 11:36
BigSimmo pushed a commit that referenced this pull request Jul 29, 2026
Confirms the squash merge landed by content: the diff between squash
commit f4a3f50 and branch tip bebc6c0 is empty, so nothing was orphaned
by the auto-merge race despite four concurrent pushes during the merge
window. Also records that the coverage follow-ups were renumbered
#98/#99 -> #106/#107 (main claimed #98-#105 concurrently) with both
rows verified intact, and that the three red `PR required` results were
concurrency cancellations rather than defects.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012YRCXgX4AWZ579bKN6sk6b
BigSimmo pushed a commit that referenced this pull request Jul 29, 2026
`Static PR checks` went red on f6a13cd: the branch-review-ledger guard found
2 exact duplicate records at lines 1276/1277, repeating the ref/HEAD/scope of
lines 1272/1273.

This is ledger #88's exact watch condition. `docs/branch-review-ledger.md`
carries `merge=union` in .gitattributes so concurrent appends survive, but a
same-hunk merge can keep both the incoming and existing copy. Repeated
origin/main syncs on this PR duplicated two PR #1383 babysit records from
claude/test-coverage-analysis-2vcd8a.

Verified both pairs byte-identical (664 and 352 chars) before touching
anything, then removed only the later copies. The ledger contract allows
removing exact duplicates and forbids rewriting surrounding records, so the
diff is 2 deletions and 0 additions.

check:branch-review-ledger now passes: 1231 table records, union merge active,
six cells each, no conflict markers, mojibake, heading records, or duplicates.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01361jh3eYVjJCzXWjAhdZiF
BigSimmo added a commit that referenced this pull request Jul 29, 2026
…low-clone trap (#1387)

Documentation and ledger bookkeeping only; the whole diff is
docs/branch-review-ledger.md and docs/outstanding-issues.md.

Records the PR #1383 prlanded verification (squash-vs-tip content diff empty,
nothing orphaned by the auto-merge race), carries the babysit closeout row that
the squash left behind, and files the branch-cleanup review with a per-candidate
row keyed to each branch's own HEAD.

New issue #109: remote sessions clone shallow, which silently invalidates every
merge-base, cherry-pick and ahead/behind result. A sweep in that state wrongly
reported 90 of 91 branches as carrying unmerged work; git fetch --unshallow
corrected it.

#108 records the ordering constraint that makes the cleanup executable: append
each completed branch-cleanup ledger row BEFORE deleting the ref, because
resolveHead refuses a HEAD that is no longer a commit in the repository, and the
n/a escape hatch never satisfies hasCompletedCleanupReview.

Clinical governance preflight not applicable: classifyPullRequestFiles reports
clinicalRisk false for a docs-only diff.
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.

3 participants