perf: reduce Therapy startup and sidebar layout work - #1489
Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 39 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 (19)
📝 WalkthroughWalkthroughThis PR splits the Therapy Compass dataset into content-addressed home, index, and full catalogue assets with legacy aliases, updates the build script, bindings, and data hook to select the correct catalogue per route, and adds caching rules. It also hardens the bundle-budget script and CI/Docker install steps, removes sidebar transition logic, adds viewport-resize handling to scroll-hide behavior, and updates review documentation. ChangesTherapy catalogue assets
Build and CI hardening
Sidebar transition removal
Viewport-resize scroll-hide handling
Review documentation
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant TcProvider
participant useTherapyData
participant THERAPY_CATALOGUE_ASSETS
participant TherapyCatalogueJSON
TcProvider->>useTherapyData: Select home, index, or full catalogue
useTherapyData->>THERAPY_CATALOGUE_ASSETS: Resolve hashed asset filename
useTherapyData->>TherapyCatalogueJSON: Request selected JSON asset
sequenceDiagram
participant CI
participant CheckBundleBudgetScript
participant runBundleBudgetCheck
participant exitProcess
CI->>CheckBundleBudgetScript: Run script directly with timeout
CheckBundleBudgetScript->>runBundleBudgetCheck: Measure chunks and validate budget
runBundleBudgetCheck->>exitProcess: Return status code
exitProcess->>CI: Terminate process with code
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
Comment |
![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: 835a66af0e
ℹ️ 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".
|
@codex review PR |
BigSimmo
added
the
skip-branch-sync
|
@cursoragent run bugbot review for PR and resolve merge conflict |
|
Merge conflict cleared and Bugbot triage finished on PR #1489. Tip is Merge conflictOnly conflict was
Bugbot reviewNo unresolved
Checks
  |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
There was a problem hiding this comment.
Pull request overview
This PR optimizes Therapy Compass startup and sidebar layout work by introducing content-addressed (hashed) therapy catalogue assets with a generated manifest, loading smaller route-appropriate payloads (home vs search vs detail), and removing a costly grid-template-columns transition during sidebar toggles.
Changes:
- Generate and consume a manifest-driven set of hashed Therapy Compass catalogue JSON assets (plus a compact home projection) to reduce client parse/work and enable immutable caching.
- Route-select the appropriate therapy dataset (
home/index/full) viauseTherapyData+ Therapy Compass bindings, and update tests/fixtures accordingly. - Remove the sidebar grid-column transition wiring from the main shell owners to avoid layout animation jank.
Reviewed changes
Copilot reviewed 13 out of 17 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/ui-therapy-nav-scroll.spec.ts | Updates offline fixture allowlist to accept hashed therapy catalogue asset filenames. |
| tests/ui-route-coverage.spec.ts | Same fixture allowlist update for route-coverage Playwright runs. |
| tests/therapy-compass-pathways.test.ts | Switches pathway integrity test to read the manifest-provided full catalogue asset. |
| tests/therapy-compass-mode-wiring.test.ts | Asserts hashed assets exist, verifies legacy aliases match, and updates loader expectations to manifest usage. |
| tests/therapy-compass-data-recovery.dom.test.tsx | Updates fetch mocks/assertions to use manifest-driven asset names (home/full). |
| src/components/therapy-compass/data/use-therapy-data.ts | Adds home catalogue option and loads catalogue JSON via THERAPY_CATALOGUE_ASSETS[...] (manifest). |
| src/components/therapy-compass/data/generated-assets.ts | New generated manifest exporting hashed therapy asset filenames. |
| src/components/therapy-compass/bindings.tsx | Selects home vs index vs full catalogue based on Therapy Compass route/screen. |
| src/components/ClinicalDashboard.tsx | Removes sidebar grid-column transition gating (drops useSidebarColumnTransitionReady). |
| src/components/clinical-dashboard/global-search-shell.tsx | Same: removes grid-column transition gating for the standalone search shell. |
| scripts/build-therapies-index.mjs | Generates compact home projection, writes hashed assets + TS manifest, and validates assets via --check. |
| public/therapy-compass-data/therapies-home.json | Adds the compact home projection dataset. |
| next.config.ts | Adds immutable caching headers for content-addressed therapy catalogue assets. |
| docs/outstanding-issues.md | Updates the Therapy Compass perf tracking entry to reference the hashed asset name. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Resolve docs/outstanding-issues.md by keeping main's open queue (HEAD-only rows already archived on main) and preserving this PR's #117 hashed catalogue filename note. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
|
Temporarily paused during the owner-authorized ordered merge sweep so |
|
@cursoragent this PR will be re-opened soon. Please resolve any failing CI, run bugbot and resolve any bugbot comments against current repo and fix any merge conflicts and also fix any errors and address and fix all comments or suggestions that are likely to improve the PR and Codebase. Please implement smallest required fix according to best code practices. Check your work. Leave this PR closed for now but still make all these changes and run the review. I want it ready to go for when I re-open it. |
|
PR #1489 is ready to reopen and still closed. Branch tip is Done
Checks
NoteGitHub may still show the closed PR head as |
Resolve docs/outstanding-issues.md by keeping main's open queue (HEAD-only open rows already archived on main) and preserving this PR's #117 home/index catalogue split note. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Serve unversioned therapy catalogue aliases with must-revalidate so deployment-straddling clients pick up regenerated bytes, and delete the unused sidebar column-transition hook left after import removal. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Guard the immutable hashed catalogue headers and the must-revalidate compatibility-alias policy so a future next.config change cannot silently collapse them. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
|
PR #1489 reopen prep is done on Merge conflict
Bugbot / review threads
Checks run
On reopenGitHub’s closed-PR head may still show |
|
@CodeRabbit please review PR |
|
✅ Action performedReview finished.
|
|
@coderabbitai please run a new review of this closed PR for me |
|
✅ Action performedFull review finished. Your included review limit is currently reached under our Fair Usage Limits Policy. Your recent PR review activity is in the 95th percentile or higher among CodeRabbit users, so adaptive limits apply. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 41 minutes. |
Deep fix: Build
|
Behind-but-clean (merge-tree clean); GitHub DIRTY blocked pull_request CI. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Review follow-up: always route unexpected throws through exitProcess, and only drop fixture buffers after the violation check so failure reporting cannot race a cleared buffer. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Review + stress test — bundle-budget hang fixVerdict: Fix is sound. No P0–P2 remaining in this delta. Tip What was wrong (confirmed)Build printed bundle-budget success, then never exited → 25m job timeout → cancelled. “Re-run failed jobs” skipped cancelled Build → Fix review
Stress evidence (local)
Residual riskIf Synced |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@public/therapy-compass-data/therapies-home.f68f0bb9623bc42a.json`:
- Around line 3928-3929: Update the upstream therapy catalogue record that
generates the task-centred-practice entry so its slug and name use plain “k”
rather than the \u006b escape; ensure the generated therapies-home asset emits
task-centred-practice and Task-centred practice, and verify existing hard-coded
routes, tests, or links continue resolving that slug.
In `@public/therapy-compass-data/therapies-index.7cd59ff6765d15f8.json`:
- Around line 720-724: Stop deriving modality from tags in the therapy index
generator, and inspect all route, filter, and facet consumers before choosing
removal versus curated values. Update the generator and affected records so
clinically unsupported modality values are omitted or replaced only with
source-backed values, including the ECT, rTMS, cognitive stimulation, MBT, TFP,
psychoanalysis, and psychodynamic entries; preserve conservative behavior when
no curated value exists.
In `@tests/bundle-budget.test.ts`:
- Around line 50-84: Update both exitProcess calls in the exitProcess tests to
cast their mock exitImpl and stdout objects to the inferred option types,
matching the existing setTimer cast where needed. Keep the test behavior and
mock implementations unchanged while ensuring both strict TypeScript call sites
compile.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e0e1ffd9-2289-4342-b08b-7ec6f34a4baf
📒 Files selected for processing (20)
.github/workflows/ci.ymlDockerfileDockerfile.workerdocs/branch-review-ledger.mddocs/outstanding-issues.mdpublic/therapy-compass-data/therapies-home.f68f0bb9623bc42a.jsonpublic/therapy-compass-data/therapies-home.jsonpublic/therapy-compass-data/therapies-index.7cd59ff6765d15f8.jsonscripts/build-therapies-index.mjsscripts/check-bundle-budget.mjssrc/components/clinical-dashboard/global-search-shell.tsxsrc/components/clinical-dashboard/use-hide-on-scroll.tssrc/components/therapy-compass/bindings.tsxsrc/components/therapy-compass/data/generated-assets.tssrc/components/therapy-compass/data/use-therapy-data.tstests/bundle-budget.test.tstests/therapy-compass-data-recovery.dom.test.tsxtests/therapy-compass-mode-wiring.test.tstests/ui-phone-scroll-page-owned.spec.tstests/use-hide-on-scroll.test.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- src/components/therapy-compass/data/generated-assets.ts
- src/components/clinical-dashboard/global-search-shell.tsx
- tests/therapy-compass-mode-wiring.test.ts
- src/components/therapy-compass/bindings.tsx
- src/components/therapy-compass/data/use-therapy-data.ts
- scripts/build-therapies-index.mjs
- tests/therapy-compass-data-recovery.dom.test.tsx
Resolve conflicts in use-hide-on-scroll and the Services phone-scroll poll by keeping main's range-change hold and this branch's viewportHeightChanged rebase (plus visualViewport resize listeners). Update the header contract test to pin the merged resize wiring. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
|
@cursoragent please thoroughly review these failing CI so they don't occur again and address all comments logically |
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
|
Fixed the CI-blocking issue and cleared the open review threads on PR #1489 (tip CI failure (won’t recur)Static PR typecheck failed on Fix: Comments addressed
All three threads resolved (inline replies 403; dispositions in the PR summary comment). Left aloneClinical catalogue modality/settings cleanup, auto-merge, merge to main. Hosted CI is re-running on the new tip. |
exitProcess now declares injectable mock options via JSDoc so Vitest mocks typecheck under strict tsc (the Static PR failure on bundle-budget tests). The therapies generator no longer mid-word-rewrites every sk- sequence into s\u006b-; only OpenAI-key-shaped tokens are escaped, so task-centred stays plain in pretty-printed home/index assets. Regenerated hashed catalogues and added regression coverage. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
CI + review-comment pass (
|
| Thread | Disposition |
|---|---|
exitProcess mocks (Critical) |
Fixed in 3e56dd91 |
\u006b in task-centred |
Fixed — generator mid-word sk- rewrite mangled task-centred; now only key-shaped \bsk-[A-Za-z0-9]{16,} is escaped; home/index regenerated (211dab55 / 4b4ee988) |
| modality derived from tags (Major) | No change — pre-existing on main catalogue source; out of scope for this perf PR |
Inline replies are 403 for the agent token; threads were resolved via GraphQL. Dispositions recorded here.
Not changed
- Clinical modality/settings/tag cleanup (catalogue-data PR)
- Provider-backed gates / auto-merge / merge to main
Merge left to you once required CI on 3e56dd91 is green.
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
|
@cursoragent this keeps failing... please do a thorough deep dive to ensure no more failures with CI |
|
Taking a look! |
GitHub reported DIRTY/CONFLICTING while merge-tree was clean; branch was one commit behind main. Sync so pull_request CI can build refs/pull/1489/merge. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
CI deep-dive unblock (
|
Real conflict in docs/scripts-index.md after main added the outstanding-issues writer. Regenerated inventory (198 scripts / 212 npm scripts) so PR mergeability can build refs/pull/1489/merge again.
CI deep-dive — unblocked (
|
Resolves the duplicate #286 that made the PR un-mergeable. Main's in-page-nav series claims #286-#288; this branch independently created its own #286 for the authorizationHeader casing trap. Kept both sides and renumbered the branch's row to #289, bumping issues:next-id to 290. The auto-merge silently dropped the branch's #286 detail row rather than conflicting on it -- only the recommended-queue hunk raised a marker. That row is restored as #289. This is the silent row-loss mode archive row #112 describes: the guard catches duplicate ids but not a vanished one. Also addresses review findings on the handover brief: - Require the Clinical Governance Preflight for Phase 3 regardless of what scripts/pr-policy.mjs classifies. AGENTS.md:257 keys the requirement to behaviour -- source rendering and document access -- and Phase 3 changes both. The classifier not matching src/components/** is the PR #1489 gap recorded at pr-policy.mjs:62, not an exemption. - Resolve the scope contradiction: the brief claimed everything stays in src/components/document-viewer/** while Task 3 authorised wiring src/app/api/images/signed-urls/route.ts, which is clinical-risk. That is now an explicit, discouraged scope exception. - Correct the signed-URL warning. Both identity bugs are already fixed on main (authorizationIdentity() helper, identity in the dedupe key, cache write outside the shared promise, tests/auth-signed-url-cache.dom.test.tsx). The brief now says so and requires that coverage stay green, rather than reading as an open disclosure with a "consider" remedy. - Drop a literal NUL byte that had landed in the prose, and remove the trailing-space code span (MD038). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ER2xPFzPzoS8fAxkgHC8yo
Summary
SENTRY_DSNand scrubs request, user, body, breadcrumb, and clinical content before export.documents.metadata.docs/error-tracking.md.RAG impact: no retrieval behaviour change — prompt presentation only adds conservative normalized source-governance metadata; retrieval, ranking, and source selection are unchanged.
Verification
npm run verify:pr-localtests/error-tracking.test.ts,tests/rag-source-governance-prompt.test.ts, andtests/source-metadata.test.tsnpm run eval:rag:offline— 36/36 golden retrieval casesnpm run check:production-readiness:cireturned READY (offline; expected missing-env warnings)npm run buildsucceeded for production instrumentation wiringorigin/mainand resolved Codex Cloud git-remote helper conflicts;git merge-treevsorigin/mainis cleanRisk and rollout
main, removeSENTRY_DSNand restart services if observability was enabled, and confirm prompts no longer emit the Source governance line.SENTRY_DSN; no browser DSN, tracing, or source-map upload is configured.Clinical Governance Preflight
Clinical KB Database(sjrfecxgysukkwxsowpy)Notes
{}/index-only governance metadata remains unrecorded in prompts; partial sibling fields use neutralunknownrather than inventing adverseunverified; explicit storedclinical_validation_status: "unverified"from upload is preserved.