fix: remediate the PR #1489 review findings - #1532
Conversation
…ues, gate clinical data Three findings from the PR #1489 review, all in the generator/policy layer. 1. `modality` was inferred, not curated. The source catalogue derives it from each record's own tag list: all 205 records carry one, every value is also present in that record's `tags`, and the catalogue collapses to CBT/ACT/DBT. That mislabels the treatments it cannot describe — ECT and rTMS as "ACT", Psychoanalysis and Psychodynamic Psychotherapy as "CBT", MBT and TFP as "DBT" — while rendering as a curated chip on the detail and recommend screens and scoring related-therapy selection. Emit it only when the source curates a value that is not already a tag; consumers already treat null as unknown. Removing it is provably search-neutral: `src/lib/therapies.ts` scores with a boolean `haystack.includes(token)`, not term frequency, and every modality value is already contributed by `tags.join(" ")` in the same haystack. 2. Superseded content-addressed assets were never pruned. Each regeneration mints new hashed filenames, so every data revision stranded the previous full catalogue (~2.5 MB) plus both projections permanently, in git history and in every image; PR #1489 stranded two inside one pull request and needed a hand-deletion commit. Pruning keeps a one-deploy grace generation, because dropping the previous hash immediately would 404 a client whose bundle still names it — the deployment-straddling failure the unversioned aliases exist to prevent. `useTherapyData` now also falls back to the alias, covering bundles older than the grace window. Rotation is per-kind and content-driven so a repeat local run cannot retire a generation that is still deployed. 3. `classifyPullRequestFiles` treated clinical reference datasets as non-clinical. PR #1489 shipped 205 therapy records carrying per-record recommendation prose with `clinicalRisk: false`, so no governance preflight was ever required. `src/data/**`, `data/**` and `public/therapy-compass-data/**` are clinical output even with no code diff. Also widen the false-key escaper: an alphanumeric-only tail never matched `sk-proj-…`, whose hyphen sits four characters in, leaving unescaped the key shape most likely to be pasted into source. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The `viewportHeightChanged` guard added in PR #1489 sat above the `offset <= topRevealOffset` branch, so a resize landing while the chrome was hidden and the scroller was clamped to the top returned `hidden: true` and stranded the chrome off-screen at offset 0 until the next scroll. The top reveal band is an absolute layout contract; a viewport change does not stop the reader being at the top of the range. Reordered, with a test at offset 0 that the existing 480/500 case never covered. The guard also keyed on `window.innerHeight` while the listener fired on `visualViewport` resize, so it could not engage for the Safari toolbar collapse its own comment named first. Measure `visualViewport.height` instead, rounded so sub-pixel jitter during a toolbar animation does not register as a resize on every frame, and fall back to `innerHeight` for jsdom and older engines. Also record the deliberate removal of the sidebar grid-track transition at both call sites. Animating `grid-template-columns` relayouts the whole shell every frame and needed a mount-gating hook in both shells to avoid animating from the default track width; the snap is the point, not a missing transition. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The 3-minute step timeout added in PR #1489 exists so a stuck exit fails as `failure` rather than being cancelled at the job timeout, which "Re-run failed jobs" will not re-run. The check gzips every chunk once and finishes in seconds, so the ceiling only needs to sit well below the job timeout — 3 minutes left no margin for a slow or contended runner and would turn a healthy build red. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Ledger row for the review at head 67d5cb9 (approved with follow-ups, merged 9451482), and four open items: therapy modality now null on all 205 records pending curation or removal, the sourceChanged guard's ordering defect, the catalogue aliases duplicating 2.53 MB, and pr-policy not flagging operational risk bundled with clinical or UI risk. The "search still loads the full corpus" follow-up is already tracked by #117 and was not duplicated. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in:29 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 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 configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (21)
✨ Finishing Touches📝 Generate docstrings
Comment |
🔒 Upgrade RequiredPrivate repository analysis requires Pro or Enterprise. Upgrade: https://ecc.tools/pricing?plan=pro ECC Tools keeps the core app open, and puts private repos, team features, and enterprise controls behind paid tiers. |
PR Summary by QodoFix therapy catalogue modality, prune hashed assets, and tighten clinical gating
AI Description
Diagram
High-Level Assessment
Files changed (19) |
BigSimmo
commented
Jul 31, 2026
@cursoragent Work the current open PR end-to-end. Confirm the PR number and GitHub head first from context. If more than one open PR could apply, stop and say which one you would use and why. Fetch and start from the remote tip that matches that GitHub head. If the named branch ref is missing or stale, use the PR head ref. Preserve unrelated local WIP, including any local-only ledger commits; do not discard dirty work, and do not treat a local-ahead commit as the reviewed tip. Do not merge the PR, force-push, rebase, or arm auto-merge unless I explicitly ask. No provider-backed gates without separate approval. If you cannot push or resolve threads, diagnose and comment only; if inline replies fail, resolve when possible and put dispositions in the summary comment. If auto-merge is already armed, push only for a real blocker, and avoid pushes that would cancel in-flight required CI unless the push itself clears that blocker. If the PR is already merged or closed: confirm the head and merge commit, note required-CI outcome, post one summary, and stop. Goal: deep review plus Bugbot, fix actionable issues with the smallest correct changes, clear merge / required-CI / thread blockers, run strong local offline verification, push fixes, append the review ledger, and post one PR summary. Prefer thoroughness over speed. Regenerate large assets only when a fix requires it; then run the asset check and keep compatibility aliases byte-identical where the repo uses them. Snapshot the GitHub head SHA: tip, base, behind/ahead, mergeable state, merge-tree versus origin/main (real conflict versus behind-but-clean), required checks on that tip including Production UI when selected, advisory separately, unresolved actionable threads. Missing checks while dirty are not green. If the tip moves mid-work, re-snapshot and continue from the new head. Ledger-lookup against that GitHub head under the heavy review-and-fix scope for this PR. Already reviewed at this head with clean merge-tree, green required checks, and no new actionable threads → summarize, comment, stop unless I asked for a fresh superseding pass. Follow the repo review protocol. Unblock once: real conflict → merge origin/main (prefer main’s shared queues; keep this PR’s notes); behind-but-clean → one sync, late if required CI is in flight; clean → leave. Labels like skip-branch-sync do not block a manual sync needed to clear a real blocker. No sync thrash. After any sync or push, re-snapshot tip, merge-tree, and required checks before declaring done. Dedupe the ledger if a merge touched it. Review high-confidence delta risks only. Separate PR-introduced defects from pre-existing re-emitted debt. For generated assets, review contracts, aliases, cache, and manifests—not every generated line. If protected RAG or ranking surfaces are touched, say so before editing; fix PR-body policy text only when wrong or missing. Ignore bot noise. No nit spam or broad rewrites. Escalate verification by touched risk: clinical / RAG / privacy / migrations / auth → domain check plus production-readiness when warranted; UI / phone-chrome / routing / styling → ensure plus phone-chrome or UI gates when warranted; generated assets or docs inventory/links → asset or docs checks when warranted. Fix P0/P1 always; clear scoped P2 when locally provable; else disposition and resolve. Required-check failures on this tip only; ignore advisory. Prefer reverting a bad autofix commit. After fixes: format and commit before push; smallest targeted proof; repo cheap gate; repo PR-local gate. No release, lighthouse, live eval, or live provider gates without approval. Push only this PR’s fix commits. Append ledger for the final GitHub head under the heavy scope. Never push a ledger-only tip—include ledger in a real fix push, or leave a no-change ledger append local and unpushed. One PR comment: tip, sync/merge-tree, fixed versus dispositioned, threads, required CI, decisive local gate lines, residual risks; merge left to me. Inline only for remaining human-needed P0/P1. Stop when merge-tree is clean on the current GitHub tip, actionable threads are clear, required checks are green or clearly in progress, heavy gates for scope passed, summary posted, merge left to me. |
PR #1532 worked end-to-end at tip Done
State now
  |
BigSimmo
commented
Jul 31, 2026
@CodeRabbit open PR review |
Code Review by Qodo🐞 Bugs (0)📘 Rule violations (0)📎 Requirement gaps (0)Great, no issues found!Qodo reviewed your code and found no material issues that require reviewTo customize comments, go to the Qodo configuration screen, or learn more in the docs. |
Detail and recommend load catalogue: "full", not the index projections. curatedModality was only applied to the thin indexes, so the chips and related-therapy same-modality boost still used ECT→ACT and similar mislabels. Write the curated full payload, pin the echo invariant on THERAPY_CATALOGUE_ASSETS.full, and align the grace-window comments with content-driven rotation.
BigSimmo
commented
Jul 31, 2026
@cursoragent Work the current open PR end-to-end. Confirm the PR number and GitHub head first from context. If more than one open PR could apply, stop and say which one you would use and why. Fetch and start from the remote tip that matches that GitHub head. If the named branch ref is missing or stale, use the PR head ref. Preserve unrelated local WIP, including any local-only ledger commits; do not discard dirty work, and do not treat a local-ahead commit as the reviewed tip. Do not merge the PR, force-push, rebase, or arm auto-merge unless I explicitly ask. No provider-backed gates without separate approval. If you cannot push or resolve threads, diagnose and comment only; if inline replies fail, resolve when possible and put dispositions in the summary comment. If auto-merge is already armed, push only for a real blocker, and avoid pushes that would cancel in-flight required CI unless the push itself clears that blocker. If the PR is already merged or closed: confirm the head and merge commit, note required-CI outcome, post one summary, and stop. Goal: deep review plus Bugbot, fix actionable issues with the smallest correct changes, clear merge / required-CI / thread blockers, run strong local offline verification, push fixes, append the review ledger, and post one PR summary. Prefer thoroughness over speed. Regenerate large assets only when a fix requires it; then run the asset check and keep compatibility aliases byte-identical where the repo uses them. Snapshot the GitHub head SHA: tip, base, behind/ahead, mergeable state, merge-tree versus origin/main (real conflict versus behind-but-clean), required checks on that tip including Production UI when selected, advisory separately, unresolved actionable threads. Missing checks while dirty are not green. If the tip moves mid-work, re-snapshot and continue from the new head. Ledger-lookup against that GitHub head under the heavy review-and-fix scope for this PR. Already reviewed at this head with clean merge-tree, green required checks, and no new actionable threads → summarize, comment, stop unless I asked for a fresh superseding pass. Follow the repo review protocol. Unblock once: real conflict → merge origin/main (prefer main’s shared queues; keep this PR’s notes); behind-but-clean → one sync, late if required CI is in flight; clean → leave. Labels like skip-branch-sync do not block a manual sync needed to clear a real blocker. No sync thrash. After any sync or push, re-snapshot tip, merge-tree, and required checks before declaring done. Dedupe the ledger if a merge touched it. Review high-confidence delta risks only. Separate PR-introduced defects from pre-existing re-emitted debt. For generated assets, review contracts, aliases, cache, and manifests—not every generated line. If protected RAG or ranking surfaces are touched, say so before editing; fix PR-body policy text only when wrong or missing. Ignore bot noise. No nit spam or broad rewrites. Escalate verification by touched risk: clinical / RAG / privacy / migrations / auth → domain check plus production-readiness when warranted; UI / phone-chrome / routing / styling → ensure plus phone-chrome or UI gates when warranted; generated assets or docs inventory/links → asset or docs checks when warranted. Fix P0/P1 always; clear scoped P2 when locally provable; else disposition and resolve. Required-check failures on this tip only; ignore advisory. Prefer reverting a bad autofix commit. After fixes: format and commit before push; smallest targeted proof; repo cheap gate; repo PR-local gate. No release, lighthouse, live eval, or live provider gates without approval. Push only this PR’s fix commits. Append ledger for the final GitHub head under the heavy scope. Never push a ledger-only tip—include ledger in a real fix push, or leave a no-change ledger append local and unpushed. One PR comment: tip, sync/merge-tree, fixed versus dispositioned, threads, required CI, decisive local gate lines, residual risks; merge left to me. Inline only for remaining human-needed P0/P1. Stop when merge-tree is clean on the current GitHub tip, actionable threads are clear, required checks are green or clearly in progress, heavy gates for scope passed, summary posted, merge left to me. |
Taking a look! |
🧩 Analysis chain🏁 Script executed: #!/usr/bin/env bashset -euo pipefail
repo="BigSimmo/Database"
pr="1532"echo"== Pull request snapshot =="
gh pr view "$pr" --repo "$repo" --json number,state,url,title,headRefName,headRefOid,baseRefName,baseRefOid,isDraft,mergeable,mergeStateStatus,reviewDecision,statusCheckRollup,labels
echoecho"== Changed files and commits =="
gh pr view "$pr" --repo "$repo" --json files,commits --jq '{files: [.files[] | {path, additions, deletions}], commits: [.commits[] | {oid, messageHeadline}]}'echoecho"== Reviews and review comments =="
gh api "repos/$repo/pulls/$pr/reviews?per_page=100" --paginate \
--jq '.[] | {id, user: .user.login, state, submitted_at, body}'||true
gh api "repos/$repo/pulls/$pr/comments?per_page=100" --paginate \
--jq '.[] | {id, user: .user.login, path, line, side, in_reply_to_id, created_at, body}'||trueechoecho"== Local checkout state =="
git status --short --branch
echo"HEAD=$(git rev-parse HEAD)"
git remote -v
git branch -vv
echoecho"== Remote PR refs and main =="
git ls-remote origin "refs/pull/$pr/head""refs/heads/main""refs/heads/*"| head -n 80Length of output: 7218 🏁 Script executed: #!/usr/bin/env bashset -euo pipefail
base="ed849c7f04401536e1324688cf5d28b7af64e0b0"
head="b4e97e4774468707e67f30e3d13e48be8de91ca3"echo"== Merge base and ahead/behind =="
merge_base="$(git merge-base "$base""$head")"printf'merge_base=%s\n'"$merge_base"
git rev-list --left-right --count "$base...$head"echoecho"== Merge-tree conflict paths =="
git merge-tree --write-tree "$base""$head">/tmp/pr1532-merge-tree.out 2>/tmp/pr1532-merge-tree.err ||true
cat /tmp/pr1532-merge-tree.err
grep -nE 'CONFLICT|<<<<<<<|=======|>>>>>>>' /tmp/pr1532-merge-tree.out ||true
sed -n '1,240p' /tmp/pr1532-merge-tree.out
echoecho"== Changed handwritten hunks =="
git diff --unified=60 "$base...$head" -- \
scripts/build-therapies-index.mjs \
scripts/lib/escape-false-openai-key-signatures.mjs \
scripts/pr-policy.mjs \
src/components/ClinicalDashboard.tsx \
src/components/clinical-dashboard/global-search-shell.tsx \
src/components/clinical-dashboard/use-hide-on-scroll.ts \
src/components/therapy-compass/data/generated-assets.ts \
src/components/therapy-compass/data/use-therapy-data.ts \
tests/escape-false-openai-key-signatures.test.ts \
tests/therapy-compass-data-recovery.dom.test.tsx \
tests/therapy-compass-mode-wiring.test.ts \
tests/therapy-compass-pathways.test.ts \
tests/use-hide-on-scroll.test.ts \
.github/workflows/ci.yml \
docs/outstanding-issues.md
echoecho"== AST outlines for changed implementation files =="
ast-grep outline scripts/build-therapies-index.mjs --items all --view expanded
ast-grep outline scripts/pr-policy.mjs --items all --view expanded
ast-grep outline src/components/clinical-dashboard/use-hide-on-scroll.ts --items all --view expanded
ast-grep outline src/components/therapy-compass/data/use-therapy-data.ts --items all --view expandedLength of output: 201968 🏁 Script executed: #!/usr/bin/env bashset -euo pipefail
base="ed849c7f04401536e1324688cf5d28b7af64e0b0"
head="b4e97e4774468707e67f30e3d13e48be8de91ca3"
merge_base="$(git merge-base "$base""$head")"echo"== Merge-tree conflict paths =="
git merge-tree --write-tree --name-only --messages "$base""$head"2>&1||trueechoecho"== Paths changed by main since the branch point and also by this PR =="
comm -12 \
<(git diff --name-only "$merge_base..$base"| sort) \
<(git diff --name-only "$merge_base..$head"| sort)echoecho"== Full-catalogue modality evidence =="
python3 - <<'PY'import jsonfrom pathlib import Pathroot = Path(".")full = json.loads((root / "public/therapy-compass-data/therapies.json").read_text())index = json.loads((root / "src/data/therapies-index.json").read_text())names = {"ECT", "rTMS", "Psychoanalysis", "Psychodynamic Psychotherapy"}for label, records in (("full-alias", full), ("server-index", index)): by_name = {record.get("name"): record for record in records} print(f"{label}: records={len(records)}") for name in sorted(names): record = by_name.get(name) print(f" {name}: modality={record.get('modality') if record else '<missing>'!r}, tags={record.get('tags') if record else '<missing>'!r}") echoes = [ record["name"] for record in records if record.get("modality") and record.get("modality") in record.get("tags", []) ] print(f" modality echoes tags: {len(echoes)}")PYechoecho"== Direct modality consumers and full-catalogue route selection =="
rg -n -C 4 --glob '*.{ts,tsx}' \
'(?:\.modality\b|modality:|catalogue:\s*"full"|screen === "(?:search|recommend|compare)")' \
src/components src/lib tests | head -n 500
echoecho"== Full asset generation and asset manifest context =="
sed -n '150,280p' scripts/build-therapies-index.mjs
sed -n '1,180p' src/components/therapy-compass/data/use-therapy-data.tsLength of output: 2548 🏁 Script executed: #!/usr/bin/env bashset -euo pipefail
base="ed849c7f04401536e1324688cf5d28b7af64e0b0"
head="b4e97e4774468707e67f30e3d13e48be8de91ca3"
merge_base="$(git merge-base "$base""$head")"
git diff --name-only "$merge_base..$base"| sort > /tmp/pr1532-base-paths.txt
git diff --name-only "$merge_base..$head"| sort > /tmp/pr1532-head-paths.txt
echo"== Paths changed by main since the branch point and also by this PR =="
comm -12 /tmp/pr1532-base-paths.txt /tmp/pr1532-head-paths.txt ||trueechoecho"== Full-catalogue modality evidence =="
python3 - <<'PY'import jsonfrom pathlib import Pathroot = Path(".")full = json.loads((root / "public/therapy-compass-data/therapies.json").read_text())index = json.loads((root / "src/data/therapies-index.json").read_text())names = {"ECT", "rTMS", "Psychoanalysis", "Psychodynamic Psychotherapy"}for label, records in (("full-alias", full), ("server-index", index)): by_name = {record.get("name"): record for record in records} print(f"{label}: records={len(records)}") for name in sorted(names): record = by_name.get(name) if record is None: print(f" {name}: <missing>") else: print(f" {name}: modality={record.get('modality')!r}, tags={record.get('tags')!r}") echoes = [ record["name"] for record in records if record.get("modality") and record.get("modality") in record.get("tags", []) ] print(f" modality echoes tags: {len(echoes)}")PYechoecho"== Direct modality consumers and full-catalogue route selection =="
rg -n -C 4 --glob '*.{ts,tsx}' \
'(?:\.modality\b|modality:|catalogue:\s*"full"|screen === "(?:search|recommend|compare)")' \
src/components src/lib tests | head -n 500
echoecho"== Full asset generation context =="
sed -n '150,280p' scripts/build-therapies-index.mjsLength of output: 23520 🏁 Script executed: #!/usr/bin/env bashset -euo pipefail
repo="BigSimmo/Database"
head="b4e97e4774468707e67f30e3d13e48be8de91ca3"echo"== Numbered finding locations =="
nl -ba scripts/build-therapies-index.mjs | sed -n '84,125p;160,205p'
nl -ba src/components/therapy-compass/data/use-therapy-data.ts | sed -n '35,75p'
nl -ba src/components/therapy-compass/screens/detail-screen.tsx | sed -n '40,55p'
nl -ba src/components/therapy-compass/screens/recommend-screen.tsx | sed -n '108,120p'
nl -ba src/components/therapy-compass/data/select.ts | sed -n '108,123p'
nl -ba tests/therapy-compass-pathways.test.ts | sed -n '205,232p'echoecho"== GitHub Actions runs for the reviewed head =="
gh api "repos/$repo/actions/runs?head_sha=$head&per_page=100" \
--jq '.workflow_runs[] | {name, status, conclusion, event, html_url, run_started_at, updated_at}' \
||trueechoecho"== Current GitHub PR snapshot =="
gh pr view 1532 --repo "$repo" \
--json state,headRefOid,baseRefOid,mergeable,mergeStateStatus,statusCheckRollup \
--jq '{state, headRefOid, baseRefOid, mergeable, mergeStateStatus, checks: [.statusCheckRollup[] | {name, status, conclusion, workflowName}]}'Length of output: 1535 🏁 Script executed: #!/usr/bin/env bashset -euo pipefail
repo="BigSimmo/Database"
head="b4e97e4774468707e67f30e3d13e48be8de91ca3"echo"== Numbered finding locations =="
cat -n scripts/build-therapies-index.mjs | sed -n '84,125p;160,205p'
cat -n src/components/therapy-compass/data/use-therapy-data.ts | sed -n '35,75p'
cat -n src/components/therapy-compass/screens/detail-screen.tsx | sed -n '40,55p'
cat -n src/components/therapy-compass/screens/recommend-screen.tsx | sed -n '108,120p'
cat -n src/components/therapy-compass/data/select.ts | sed -n '108,123p'
cat -n tests/therapy-compass-pathways.test.ts | sed -n '205,232p'echoecho"== GitHub Actions runs for the reviewed head =="
gh api "repos/$repo/actions/runs?head_sha=$head&per_page=100" \
--jq '.workflow_runs[] | {name, status, conclusion, event, html_url, run_started_at, updated_at}' \
||trueechoecho"== Current GitHub PR snapshot =="
gh pr view 1532 --repo "$repo" \
--json state,headRefOid,baseRefOid,mergeable,mergeStateStatus,statusCheckRollup \
--jq '{state, headRefOid, baseRefOid, mergeable, mergeStateStatus, checks: [.statusCheckRollup[] | {name, status, conclusion, workflowName}]}'Length of output: 14072
Blocking findingP1 — The full Therapy Compass payload still publishes inferred Line 96 and Line 114 in The full alias has all 205 tag-derived values. For example:
Record-rich routes load this full asset. The value still renders on the detail screen at The new coverage only checks Generate the full catalogue from sanitized records, or sanitize Merge and CI state
Resolve the one documentation conflict, include the clinical fix in the same push, then re-snapshot the GitHub head and checks. Run
Failed to handle agent chat message. Please try again. ✅ Action performedReview finished.
|
🔒 Upgrade RequiredPrivate repository analysis requires Pro or Enterprise. Upgrade: https://ecc.tools/pricing?plan=pro ECC Tools keeps the core app open, and puts private repos, team features, and enterprise controls behind paid tiers. |
PR #1532 babysit summary (heavy review-and-fix)Tip: Sync
Review findings
Threads: 0 unresolved (none to fix/resolve). Bot noise ignored. Required CI (this tip)In progress after the conflict-clearing push ( Local gates (decisive lines)
Residual risks
Ledger row for this head appended locally and left unpushed (would be a ledger-only tip). Merge left to you. |
BigSimmo
commented
Jul 31, 2026
@cursoragent why does this show 42000 as code changes? |
Almost none of that is handwritten code. GitHub’s +42,071 is dominated by regenerated Therapy Compass JSON.
So ~41k of the 42k is the ~2.5 MB catalogue duplicated as alias + hash (and the index hash). GitHub counts every rewritten JSON line as an addition, even when the real clinical change is “set The meaningful diff is the generator/loader/policy/scroll-hide changes — a few hundred lines, not 42k of product logic. |
Writing the scrubbed full payload through pretty-printed syncTarget turned a modality-null scrub into ~18k-line alias + hashed-twin diffs. Restore the historical single-line JSON shape for the full catalogue, pin it in --check and tests, prune the pretty intermediate hash, and keep the deployed a9dc3ae generation as the grace previous.
🔒 Upgrade RequiredPrivate repository analysis requires Pro or Enterprise. Upgrade: https://ecc.tools/pricing?plan=pro ECC Tools keeps the core app open, and puts private repos, team features, and enterprise controls behind paid tiers. |
…to claude/pr-1489-review-786e01 Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
🔒 Upgrade RequiredPrivate repository analysis requires Pro or Enterprise. Upgrade: https://ecc.tools/pricing?plan=pro ECC Tools keeps the core app open, and puts private repos, team features, and enterprise controls behind paid tiers. |
Follow-up: fixed the ~42k “code changes” inflationTip: CauseThe full Therapy catalogue on
Fix
Diff size
Checks
Merge left to you. Ledger row for this head left local/unpushed (ledger-only tip). |
Uh oh!
There was an error while loading. Please reload this page.
#179: the full catalogue silently went from a single minified line to 18,400 pretty-printed ones when the modality scrub routed it through syncTarget. Measured +123 KB raw but only +3.4 KB gzipped, so the real cost is ~37,000 lines of git churn per data revision — set against reviewable diffs on a 205-record clinical dataset, which is probably the better trade. Recorded so the format is a decision rather than a side effect. #180: that same change made the generator write its curated output back over the file it reads as source. It is stable only because curatedModality is a fixed point, and it matters because #175 asks someone to hand-curate modality values in exactly that file — an edit the next run could discard with no gate to catch it, since --check compares against what the generator would produce. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Summary
Remediates the findings from the review of PR #1489 (merged as
945148251). No finding in that PR was P0 or P1, so this is follow-up rather than a revert. Each item is its own commit.modalitywas inferred, not curated. The source catalogue derives it from each record's own tag list: all 205 records carried one, every value was also present in that record'stags, and the catalogue collapsed to CBT/ACT/DBT. That mislabelled the treatments it cannot describe — ECT and rTMS asACT, Psychoanalysis and Psychodynamic Psychotherapy asCBT, MBT and TFP asDBT— while rendering as a curated chip on the detail and recommend screens and scoring related-therapy selection. The generator now emits the field only when the source curates a value that is not already a tag, which today meansnullfor 205/205. This defect was pre-existing onmain, not introduced by perf: reduce Therapy startup and sidebar layout work #1489; perf: reduce Therapy startup and sidebar layout work #1489 surfaced it by adding a new projection.classifyPullRequestFilestreated clinical reference datasets as non-clinical. PR perf: reduce Therapy startup and sidebar layout work #1489 shipped 205 therapy records carrying per-record recommendation prose withclinicalRisk: false, so no governance preflight was ever required for it.src/data/**,data/**andpublic/therapy-compass-data/**are clinical output even with no code diff. This PR is itself gated by that new rule.viewportHeightChangedguard outranked the top reveal band. It sat above theoffset <= topRevealOffsetbranch, so a resize landing while the chrome was hidden and the scroller was clamped to the top stranded the chrome off-screen at offset 0 until the next scroll. Reordered, with a test at offset 0 that the existing 480/500 case never covered.window.innerHeightwhile the listener fired onvisualViewportresize, so it could not engage for the Safari toolbar collapse its own comment named first. It now measuresvisualViewport.height, rounded so sub-pixel jitter during a toolbar animation does not register as a resize on every frame.sk-proj-…, whose hyphen sits four characters in, leaving unescaped the key shape most likely to be pasted into source.failurerather than being cancelled at the job timeout; the check finishes in seconds, so 3 minutes risked turning a healthy build red on a contended runner. Raised to 8.grid-template-columnstransition for measured reasons; both call sites now say so, so it is not "fixed" back as a missing transition.Two defects were found in this remediation and corrected before it left the branch. Pruning the previous hash immediately would 404 a client whose bundle still names it — the deployment-straddling failure the unversioned aliases exist to prevent, one level down — so a one-deploy grace generation was added (
THERAPY_CATALOGUE_ASSETS_PREVIOUS) plus an alias fallback inuseTherapyDatafor bundles older than that window. Rotating that window on every invocation then meant a second local re-run silently retired a generation that is still deployed, so rotation is now per-kind and content-driven; verified idempotent across three consecutive runs. The currently deployedtherapies-index.4b4ee988e0ff84e2.jsonis retained, so no live client breaks.Removing
modalityis provably search-neutral rather than merely low-risk:src/lib/therapies.tsscores with a booleanhaystack.includes(token), not term frequency, and every modality value was already contributed bytags.join(" ")in the same haystack, so the token set and every score are unchanged.This PR mixes clinical-risk paths with one operational-risk path (
.github/workflows/ci.yml). That is the same bundling pattern the #1489 review criticised, and after squash the CI timeout will not be independently revertible. It is kept together because every item is a fix to one review; the classifier gap that would flag this combination is recorded as an open item rather than fixed here.Verification
npm run verify:cheap— all 29 static gates pass;lintclean;typecheckexit 0; unit suiteTest Files 449 passed (449),Tests 4700 passed | 3 skipped (4703)npm run verify:phone-chrome— contractsTest Files 9 passed (9),Tests 116 passed; focused browser leg13 passed (1.2m); escalated full-UI leg342 passed, 2 failednpm run verify:ui— run as the escalated final leg ofverify:phone-chrome:342 passed, 2 failed, both failures analysed below and passing in isolationnpm run check:therapy-data-index—Therapy indexes are current (205 records).npm run check:pr-policy—[pr-policy] self-test passednpm run format— exit 0, committedVerification not run: npm run verify:pr-local—verify:cheapplus the fullverify:phone-chromechain (which itself runsverify:ui) were run instead and are a strict superset of the unit, lint, typecheck and browser coverageverify:pr-localwould add for this diff; the build-conditional legs are exercised by CI.The two failures inside the escalated full-UI leg are
ui-tools.spec.ts:959asserting the composer's CSSpositionisfixed, on/services/13yarnand/differentials?q=acute+confusion&focus=1&run=1, both receivingrelative. They are not from this change, on evidence rather than assertion:use-hide-on-scroll.tscontains noposition:assignment at all, so scroll-hide cannot produce that value; the assertion is hero-vs-dock composer ownership, decided byisStandaloneModeHomePath, which this diff does not touch. Re-run in isolation both pass, in 1.3s and 1.4s (5 passed (11.3s)). This is the one-frame hydration race named indocs/search-chrome-behaviour.mdinvariant 12.No provider-backed gate was run. Retrieval, ranking, selection, chunking and answer generation are untouched, so no retrieval eval applies —
RAG impact: no retrieval behaviour change — this PR touches the Therapy Compass static catalogue, the phone scroll-hide guard, the PR policy classifier and one CI step timeout; no file under src/lib/rag/**, clinical-search, retrieval-selection, ranking-config, answer-ranking, the eval harness or the golden fixture is modified, and the therapy catalogue is a standalone tool dataset that never enters retrieval candidate assembly.Risk and rollout
modalitychip disappears from the Therapy detail and recommend screens for every record, which is a visible change to clinical output and is the point of the change — an inferred label rendered as curated fact. Related-therapy scoring loses a same-modality point that was near-uniform noise (77% of records wereCBT). Search recall and ordering are unchanged, provably. The catalogue pruning and phone-chrome reorder are behaviour-preserving in the cases their tests pin.cfb18d2f4(catalogue and policy),2d001305b(phone chrome) andc600ac631(CI timeout) are independent before merge; after squash they are one commit, so a post-merge revert of a single item means reverting hunks by hand. Reverting the catalogue commit restores the inferredmodalityvalues on the next regeneration, since the source data is unchanged.public/therapy-compass-data/, and the retained previous generation plus the alias fallback mean no deployed client loses a URL it depends on.Clinical Governance Preflight
Clinical KB Database(sjrfecxgysukkwxsowpy)On the sixth item specifically: this change makes the catalogue strictly more conservative.
reviewStatusis untouched and every record remainsneeds_review; the field being suppressed is one that asserted a treatment modality the source never curated. Removing an unsupported label is the conservative direction, and no record gains a claim.On the seventh: the Therapy Compass catalogue is clinical reference content in a prototype that is explicitly not validated clinical decision support. Suppressing an inferred label reduces the decision-support surface rather than extending it, so the SaMD classification is unchanged.
Notes
67d5cb91083f9b0e9d3017816cbf68abab102688vianpm run ledger:append.docs/outstanding-issues.md(#169–#172): curate or removemodality; thesourceChangedguard's identical ordering defect; the aliases duplicating 2.53 MB; andpr-policynot flagging operational risk bundled with clinical or UI risk. The "therapy search still loads the full corpus" follow-up was not duplicated — it is already tracked as#117.