Skip to content

perf(services): keep the services snapshot out of the client route chunk#890

Merged
BigSimmo merged 14 commits into
mainfrom
claude/clinical-kb-pwa-review-asi3wb
Jul 18, 2026
Merged

perf(services): keep the services snapshot out of the client route chunk#890
BigSimmo merged 14 commits into
mainfrom
claude/clinical-kb-pwa-review-asi3wb

Conversation

@BigSimmo

@BigSimmo BigSimmo commented Jul 18, 2026

Copy link
Copy Markdown
Owner

Summary

Phase 4 of the approved plan — the measured performance pass. The audit (production build + enforced-budget reading + analyzer treemap, artifacts in the session scratchpad) found the client bundle +3.13% over its enforced 1,363,382-byte gzip baseline, with one real, evidence-backed defect:

  • The 143-line /services client home page value-imported defaultServiceSlug from @/lib/services for a single static href — compiling the ~915 KB services snapshot (~100 KB gzip) into its route chunk.
  • Fix: src/app/services/page.tsx (server) computes the slug and passes it as a prop; services-home-page.tsx builds its task cards from the prop and no longer imports the services module.
  • Measured result: +3.13% → −3.97% against the old baseline. bundle-budget.json is ratcheted down 1,363,382 → 1,309,286 bytes gzip, so the blocking CI Build job locks the win permanently.

Deliberately untouched per the audit rules: pdfjs-dist (123 KB) and cross-mode-differentials data (121 KB) are already correctly lazy; the suspected forms-catalog dashboard leak was disproved (type-only import, bundles nothing); #718's freshly-remediated paths avoided.

Verification

Verification not run: Lighthouse (dev-server churn in the container; the bundle evidence fully determined this fix set).

Clinical Governance Preflight

Not applicable — a bundling-boundary refactor of how one string reaches a client component, plus the budget baseline file. No data, retrieval, clinical output, privacy, or provider behavior changes; the services data itself still loads exactly as before on the server and via the registry API.

Risk and rollout

  • Risk: low — behavior-identical prop-threading (the href renders the same slug); the ratchet only tightens CI's ceiling and cannot affect runtime.
  • Rollback: revert the commit; the baseline reverts with it since both live in the same commit.

Notes

  • Phase 4 of the plan. Phase 3 landed as squash 44a4c511. Ledger row included in this single-batch push.
  • Two transient .next/dev generated-type corruptions from the long-lived dev server were resolved by stopping the server and setting the generated dir aside (reversible); nothing was hand-deleted.

🤖 Generated with Claude Code

https://claude.ai/code/session_01EXsJcLrbZUXwnBeG91cVo9


Generated by Claude Code

Summary by CodeRabbit

  • Performance
    • Reduced gzip-compressed bundle size by ~52 KB (1,363,382 → 1,309,274 bytes) by ensuring services data is not unintentionally included in the client-side services route bundle.
  • Bug Fixes
    • Corrected the “Check catchment” task link to use the proper default service area when no search is submitted.
  • Tests
    • Added a regression test to enforce a strict client/server import boundary for services.
  • Documentation
    • Updated the performance review record and gzip bundle-size baseline with the new measured results.

The /services client home page value-imported defaultServiceSlug from
@/lib/services, compiling the ~915 KB services snapshot (~100 KB gzip)
into its route chunk for one static href. The server page now computes
the slug and passes it as a prop. Bundle budget swings from +3.13% over
to -3.97% under the enforced baseline, which is ratcheted down to
1,309,286 bytes gzip so CI locks the win. Ledger row included.

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

supabase Bot commented Jul 18, 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 18, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

The /services page computes the default service slug server-side and passes it to ServicesHomePage, which builds task links from the prop. A regression test enforces the client import boundary, while the gzip budget and review ledger record the resulting bundle measurement.

Changes

Services bundle flow

Layer / File(s) Summary
Server-computed services task cards
src/app/services/page.tsx, src/components/services/services-home-page.tsx
The page computes the default slug server-side, passes it as an optional prop, and the component uses it to build task-card links.
Client-boundary regression coverage
tests/services-client-boundary.test.ts
A Vitest test scans client modules, resolves transitive runtime imports, and asserts that no client entry reaches @/lib/services through a value import.
Bundle budget baseline record
bundle-budget.json, docs/branch-review-ledger.md
The recorded gzip total and timestamp were updated, and the review ledger documents the audit and verification results.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

  • BigSimmo/Database#608: Updates the bundle-budget gate infrastructure that reads and enforces the baseline recorded by this PR.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly matches the main change: moving the services snapshot out of the client route chunk.
Description check ✅ Passed The description follows the template and includes summary, verification, risk/rollback, and governance notes.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/clinical-kb-pwa-review-asi3wb

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

@BigSimmo
BigSimmo marked this pull request as ready for review July 18, 2026 16:19
@BigSimmo
BigSimmo enabled auto-merge (squash) July 18, 2026 16:19

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ceabc04d75

ℹ️ 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".

Comment thread docs/branch-review-ledger.md Outdated
claude and others added 2 commits July 18, 2026 16:23
Addresses the Codex P2 on PR #890: the Reviewed HEAD cell held prose
because the row predated its own single-batch commit; it now cites the
reviewed implementation commit ceabc04, keeping the exact-hash ledger
lookup functional. Row remains keyed by PR number + branch ref.

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

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 92a92eab2e

ℹ️ 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".

Comment thread bundle-budget.json
Addresses the Codex P2 on PR #890: the ratcheted bundle budget's 10%
tolerance still sits above the pre-fix total, so it cannot catch a
re-introduction of the services-snapshot client import on its own. This
source-level boundary test fails the unit suite (inside verify:cheap)
the moment any client module value-imports @/lib/services; type-only
imports remain allowed.

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

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 57b28021a3

ℹ️ 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".

Comment thread tests/services-client-boundary.test.ts Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 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 `@tests/services-client-boundary.test.ts`:
- Line 12: Update VALUE_IMPORT_PATTERN to exclude named imports whose specifiers
are exclusively type-only, including multiline forms, while continuing to match
imports that contain runtime bindings. Preserve rejection of other runtime
imports from "`@/lib/services`".
- Line 12: Broaden VALUE_IMPORT_PATTERN to detect runtime side-effect imports
and re-exports from "`@/lib/services`" in addition to regular value imports, while
continuing to allow import type statements. Update the boundary check without
changing its intended rejection of runtime service usage.
🪄 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 Plus

Run ID: b8a08e09-d65a-4c8f-9fdd-c571dfb2d4de

📥 Commits

Reviewing files that changed from the base of the PR and between de587a2 and 57b2802.

📒 Files selected for processing (1)
  • tests/services-client-boundary.test.ts

Comment thread tests/services-client-boundary.test.ts Outdated
claude and others added 2 commits July 18, 2026 16:34
… services guard

Addresses the Codex and CodeRabbit findings on PR #890: the boundary now
walks value-import graphs from every 'use client' entry (a helper without
the directive still lands in the client chunk), treats side-effect
imports and value re-exports as runtime, and stops flagging named
type-only import clauses that compile away.

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

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d955b58836

ℹ️ 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".

Comment thread tests/services-client-boundary.test.ts Outdated
Addresses the Codex P2 on PR #890: relative specifiers like
../lib/services resolved in the graph walk but never set the
services-import flag; the guard now matches the resolved module path in
addition to the alias text.

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

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 83c9293210

ℹ️ 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".

Comment thread tests/services-client-boundary.test.ts

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 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 `@tests/services-client-boundary.test.ts`:
- Around line 15-21: The import-boundary detection must also reject dynamic
import() expressions and reliably detect "use client" directives beyond the
first 400 characters. Update the logic using SIDE_EFFECT_IMPORT_PATTERN,
FROM_STATEMENT_PATTERN, and the surrounding directive check to recognize dynamic
imports and scan the full directive prologue after leading comments and
whitespace, while preserving existing handling for static imports and exports.
- Around line 98-101: Update the isClientEntry detection in the graph.set entry
to inspect the full source prologue rather than source.slice(0, 400). Ensure a
"use client" directive remains detected when preceded by a long header comment,
while preserving the existing directive-matching behavior.
🪄 Autofix (Beta)

❌ Autofix failed (check again to retry)

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 Plus

Run ID: 368f63e5-155b-4b33-91d3-abfca2332b9c

📥 Commits

Reviewing files that changed from the base of the PR and between 57b2802 and 83c9293.

📒 Files selected for processing (1)
  • tests/services-client-boundary.test.ts

Comment thread tests/services-client-boundary.test.ts
Comment thread tests/services-client-boundary.test.ts
BigSimmo and others added 2 commits July 19, 2026 00:44
…ices guard

Addresses the round-4 Codex and CodeRabbit findings on PR #890: dynamic
import() expressions now count as runtime edges (they still emit client
JavaScript), and 'use client' detection parses the complete comment/
whitespace prologue instead of capping at 400 characters.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EXsJcLrbZUXwnBeG91cVo9
@coderabbitai

coderabbitai Bot commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

Note

Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it.

The agent ran but didn't make any changes. The issues may already be fixed or require manual intervention.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 838624535e

ℹ️ 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".

Comment thread tests/services-client-boundary.test.ts Outdated
claude and others added 3 commits July 18, 2026 16:48
Addresses the round-5 Codex finding on PR #890: dynamic imports with
webpack/Next magic comments before the specifier now count as runtime
edges.

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

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 32533572b4

ℹ️ 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".

Comment thread docs/branch-review-ledger.md Outdated
Addresses the Codex P2 on PR #890: the cell held prose after the SHA,
defeating the ledger's exact rev-parse match; context moved to the
branch/ref cell.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EXsJcLrbZUXwnBeG91cVo9
@BigSimmo
BigSimmo merged commit 1d9fdcf into main Jul 18, 2026
19 checks passed
@BigSimmo
BigSimmo deleted the claude/clinical-kb-pwa-review-asi3wb branch July 18, 2026 17:10
cursor Bot pushed a commit that referenced this pull request Jul 18, 2026
Keep both branch-review ledger rows from #890 and #871.

Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
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