Skip to content

chore(tooling): pre-commit, component/a11y tests, bundle budget, dead-code + gated error tracking#623

Closed
BigSimmo wants to merge 4 commits into
mainfrom
claude/repo-improvement-review-09945c
Closed

chore(tooling): pre-commit, component/a11y tests, bundle budget, dead-code + gated error tracking#623
BigSimmo wants to merge 4 commits into
mainfrom
claude/repo-improvement-review-09945c

Conversation

@BigSimmo

@BigSimmo BigSimmo commented Jul 13, 2026

Copy link
Copy Markdown
Owner

Summary

Adds eight developer-productivity tools, each inert / advisory / baseline-calibrated so none can fail CI on pre-existing state:

  • husky + lint-staged — pre-commit prettier --write + eslint --fix (prepare guarded with husky || true so npm ci --omit=dev in the Dockerfile stays safe; the worktree config.worktree intentionally disables hooks inside worktrees, so this fires for real commits from the main checkout).
  • .editorconfig + .github/CODEOWNERS — editor consistency and review routing on high-risk paths (supabase, env, RAG/retrieval, governance, workflows).
  • jsdom + @testing-library/react — interactive component tier as a separate vitest project (tests/**/*.dom.test.tsx); the existing node suite is untouched.
  • @axe-core/playwright — automated WCAG scans in tests/ui-accessibility.spec.ts (advisory lane; zero critical/serious violations at baseline).
  • size-limit — 1.3 MB client-JS budget wired into the CI build job and verify:release (current 1.18 MB).
  • knip — advisory dead-code / unused-dependency detection (not wired into any gate).
  • Sentry (@sentry/nextjs) — gated same-origin tunnel (/api/monitoring, DSN-match validated to prevent open relay), onRequestError + error-boundary capture, and a PII scrubber reusing logger.ts SENSITIVE_KEY. Fully no-op until a DSN is set; the SDK is tree-shaken out of the client bundle via a DefinePlugin __SENTRY_ENABLED__ literal (typeof-guarded for Turbopack dev), so the CSP is unchanged.

Verification

Ran the constituent gates individually (all green) rather than the verify:pr-local wrapper:

  • lint, typecheck, format:check — clean
  • test214 files / 1960 tests pass, 0 failures (node + new jsdom project)
  • build — exit 0; client bundle 1.18 MB < 1.3 MB budget; client-bundle secret scan passes; Sentry confirmed absent from client chunks when DSN unset
  • test:e2e:accessibility — 5/5 pass incl. new axe scans
  • check:runtime, check:github-actions, sitemap:check (regenerated for the new /api/monitoring route), brand:check, check:type-scale, check:icon-scale

No retrieval/ranking/answer-generation behavior changed, so the eval gates are N/A.

Clinical Governance Preflight

Touches privacy + production config (error tracking) and adds one API route, so completing this section:

  • Source-backed claims still require linked source verification before clinical use — unchanged (no retrieval/answer changes)
  • No patient-identifiable document workflow was introduced or expanded — none; Sentry beforeSend scrubber strips request cookies/query/body, drops user identity, and redacts PII keys before any event leaves the app
  • Supabase target remains Clinical KB Database (sjrfecxgysukkwxsowpy) — untouched
  • Service-role keys and private document access remain server-only — env.ts stays server-only; only the public Sentry DSN is client-exposed; SENTRY_AUTH_TOKEN added to the client-bundle secret scanner as defense-in-depth
  • Demo/synthetic content remains clearly separated — unchanged
  • Source metadata, review status, and outdated/unknown-source behavior remain conservative — unchanged
  • Deployment/TGA SaMD impact checked — no clinical decision-support behavior changed

Notes

  • Sentry is dormant until an operator sets NEXT_PUBLIC_SENTRY_DSN + SENTRY_DSN and rebuilds — no runtime/behavior change and zero client-bundle cost until then.
  • knip ships advisory with a baseline of ~164 unused exports / 66 types for future triage before considering promotion to a required gate.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features
    • Added browser error monitoring with secure event forwarding and privacy-focused data scrubbing.
    • Added monitoring endpoint support and improved capture of global and route-level errors.
  • Documentation
    • Updated the site map to include the monitoring API route.
  • Quality Improvements
    • Added automated bundle-size checks, code analysis, formatting, linting, and pre-commit validation.
    • Expanded automated coverage for responsive tables, event callbacks, and monitoring behavior.

…dead-code + error tracking

Adds eight developer-productivity tools, each inert / advisory / baseline-
calibrated so none can fail CI on pre-existing state:

- husky + lint-staged: pre-commit prettier --write + eslint --fix (prepare
  guarded with `husky || true` so `npm ci --omit=dev` stays safe).
- .editorconfig + .github/CODEOWNERS: editor consistency and review routing on
  high-risk paths (supabase, env, RAG/retrieval, governance, workflows).
- jsdom + @testing-library/react: interactive component tier as a SEPARATE
  vitest project (tests/**/*.dom.test.tsx); the existing node suite is unchanged.
- @axe-core/playwright: automated WCAG scans in ui-accessibility.spec.ts
  (advisory lane; zero critical/serious violations at baseline).
- size-limit: 1.3 MB client-JS budget wired into the CI build job and
  verify:release (current 1.18 MB).
- knip: advisory dead-code / unused-dependency detection (not wired into any
  gate; false positives suppressed in knip.json).
- Sentry (@sentry/nextjs): gated same-origin tunnel (/api/monitoring, DSN-match
  validated), onRequestError + error-boundary capture, and a PII scrubber that
  reuses logger.ts SENSITIVE_KEY. Fully no-op until a DSN is set; the SDK is
  tree-shaken out of the client bundle via a DefinePlugin __SENTRY_ENABLED__
  literal (typeof-guarded for Turbopack dev), so the CSP is untouched.

Verified: 214 test files / 1960 tests pass (both vitest projects, 0 failures),
typecheck/lint/format clean, production build exit 0 with bundle 1.18 MB < budget,
axe 5/5.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@supabase

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

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Adds optional Sentry browser observability with privacy scrubbing and a validated monitoring tunnel, introduces jsdom-based Vitest projects and UI tests, and adds repository editor, ownership, linting, static-analysis, bundle-size, and release-validation controls.

Changes

Sentry observability

Layer / File(s) Summary
Sentry configuration and contracts
next.config.ts, src/lib/env.ts, package.json
Configures optional Sentry bundling, build-time client gating, browser Sentry environment variables, and the Sentry Next.js dependency.
Event scrubbing and client forwarding
src/lib/logger.ts, src/lib/observability/sentry-client.ts, src/lib/observability/sentry-scrub.ts
Shares sensitive-key definitions, scrubs Sentry events, truncates exposed strings, and forwards exceptions through an injected client.
Runtime reporting and monitoring tunnel
src/app/api/monitoring/route.ts, src/instrumentation-client.ts, src/app/global-error.tsx, src/components/route-error-boundary.tsx, docs/site-map.md, scripts/check-client-bundle-secrets.mjs
Initializes gated client reporting, captures boundary errors and router transitions, validates and relays Sentry envelopes, documents the route, and blocks source-map upload tokens from client bundles.
Sentry behavior tests
tests/sentry-observability.test.ts
Tests event scrubbing, client forwarding, missing configuration, DSN mismatch rejection, and matching-envelope relay behavior.

Test infrastructure and UI validation

Layer / File(s) Summary
Vitest environment setup
vitest.config.mts, tests/setup/jsdom.setup.ts, package.json
Adds separate Node and jsdom Vitest projects, browser API setup, DOM cleanup, mock restoration, and React testing dependencies.
Component and hook interaction coverage
tests/accessible-table.dom.test.tsx, tests/use-event-callback.dom.test.tsx
Tests responsive AccessibleTable expansion and dialog behavior, plus stable callback identity and latest-handler dispatch.

Repository controls and validation

Layer / File(s) Summary
Editor and ownership conventions
.editorconfig, .github/CODEOWNERS
Defines repository editor defaults, generated-file exceptions, and ownership routing rules.
Pre-commit and static-analysis wiring
.githooks/pre-commit, .lintstagedrc.json, knip.json
Runs staged-file formatting and linting through the pre-commit hook and configures Knip analysis scope and exclusions.
Bundle budget validation
.size-limit.json, package.json
Sets a 1300 KB gzip limit for client JavaScript chunks and adds bundle-size verification to release checks.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Browser
  participant InstrumentationClient
  participant MonitoringRoute
  participant SentryIngest
  Browser->>InstrumentationClient: capture exception or router transition
  InstrumentationClient->>MonitoringRoute: POST Sentry envelope
  MonitoringRoute->>SentryIngest: validate DSN and forward envelope
  SentryIngest-->>MonitoringRoute: return upstream response
Loading

Suggested reviewers: claude

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 36.84% 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 is specific and accurately reflects the main tooling, testing, size-budget, and Sentry changes.
Description check ✅ Passed The description includes the required Summary, Verification, Clinical Governance Preflight, and Notes sections and is sufficiently detailed.
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 docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/repo-improvement-review-09945c

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

@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: 12cf134988

ℹ️ 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 on lines +47 to +48
delete event.request.query_string;
delete event.request.data;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Strip query strings from Sentry request URLs

When an error is captured on a route whose URL contains user text (this repo already accepts q/query search params on clinical flows such as the differentials pages), deleting request.query_string is not enough because Sentry's request.url commonly contains the full URL, and the current scrubber leaves it untouched. That can send raw clinical queries or auth callback parameters to Sentry despite this privacy gate; scrub event.request.url to path-only or remove its search component, and consider redacting referer the same way.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

@codex fix

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Summary

Commit / PR

  • Committed changes on the current branch with commit d30a609 (fix(observability): scrub sentry request urls).
  • Created PR record: fix(observability): scrub Sentry request URL query strings.

Testing

  • ⚠️ npm run test -- --run tests/sentry-observability.test.ts — initial run could not start because node_modules/vitest/vitest.mjs was missing under the default Node 20 environment.
  • npm run test -- --run tests/sentry-observability.test.ts
  • npm run typecheck
  • npm run lint

View task →

Comment thread src/app/api/monitoring/route.ts
@BigSimmo
BigSimmo enabled auto-merge (squash) July 14, 2026 05:19
@BigSimmo

Copy link
Copy Markdown
Owner Author

@copilot resolve the merge conflicts on this branch.

auto-merge was automatically disabled July 14, 2026 05:28

Head branch was pushed to by a user without write access

@BigSimmo

Copy link
Copy Markdown
Owner Author

@codex resolve actionable Codex review findings for this pull request and current head using the repository instructions. This is the pull request's single automatic repair pass: do not perform a fresh review, create new standalone findings, or request another review. Work only the existing unresolved Codex threads on the current head. Always fix P0 and P1 findings. For P2 and lower findings, fix only clear, scoped, low-risk issues; otherwise disposition them with a concise reason. After fixing or dispositioning a thread, reply in that thread with as the first line, followed by a concise summary; that marker authorizes the workflow to close that exact thread. If human input or new authorization is required, do not use the marker and leave the thread open with the blocker. Finish only after every actionable thread is fixed or dispositioned and closed, or explicitly left open for a human decision. Do not update the branch from main, address unrelated reviews, broaden scope, or create more than one scoped fix commit. Do not use external APIs, paid services, credentials, dependency changes, or broad refactors unless explicitly authorized. Add targeted tests where behavior changes and run the narrowest relevant validation.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@BigSimmo
BigSimmo enabled auto-merge (squash) July 14, 2026 05:35
… instrumentation.ts, env.ts, a11y spec

Conflict resolution summary:
- .github/workflows/ci.yml: keep main's check:bundle-budget (advisory warn-only gate)
- package.json scripts: keep main's postinstall/githooks system; drop husky prepare
- package.json deps: keep both @sentry/nextjs (client) and @sentry/node (server)
- src/instrumentation.ts: adopt main's typed onRequestError + @sentry/node init;
  remove the superseded initSentryServer() helper (replaced by inline init in register())
- src/lib/env.ts: use main's z.preprocess SENTRY_DSN; retain NEXT_PUBLIC_SENTRY_*
  vars used by instrumentation-client.ts
- tests/ui-accessibility.spec.ts: adopt main's testInfo-attached axe helper;
  remove the standalone axe test.describe (axe scans are now in the media smoke suite)
- .githooks/pre-commit: migrate lint-staged hook from .husky to .githooks so both
  pre-commit (lint-staged) and pre-push (guard-push) live under core.hooksPath
- .husky/pre-commit: removed (superseded by .githooks/pre-commit)
- package-lock.json: regenerated via npm install to include @sentry/nextjs
auto-merge was automatically disabled July 14, 2026 05:38

Head branch was pushed to by a user without write access

Copilot AI commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

@copilot resolve the merge conflicts on this branch.

Resolved in commit 06db701. Here's a summary of each conflict:

  • .github/workflows/ci.yml — kept main's check:bundle-budget (advisory warn-only gate)
  • package.json scripts — kept main's postinstall/.githooks hook system; removed "prepare": "husky || true" (replaced below)
  • package.json deps — kept both @sentry/nextjs (client instrumentation) and @sentry/node (server-side capture)
  • src/instrumentation.ts — adopted main's typed onRequestError with captureServerException; removed the superseded initSentryServer() helper (inline init in register() already covers it)
  • src/lib/env.ts — used main's z.preprocess SENTRY_DSN; retained NEXT_PUBLIC_SENTRY_* vars used by instrumentation-client.ts
  • tests/ui-accessibility.spec.ts — adopted main's testInfo-attached axe helper; removed the standalone axe test.describe (axe scans are now integrated into the media smoke suite)
  • .githooks/pre-commit — migrated lint-staged hook from .husky/ to .githooks/ so both pre-commit (lint-staged) and pre-push (guard-push) live under the same core.hooksPath; removed .husky/pre-commit

Verified: lint ✅, typecheck ✅, 2273 unit tests ✅

@BigSimmo
BigSimmo enabled auto-merge (squash) July 14, 2026 05:38

@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: 7

🧹 Nitpick comments (1)
tests/setup/jsdom.setup.ts (1)

12-40: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Dedupe the matchMedia/scrollIntoView stub and fix the one-time scrollIntoView guard. The browser-API stub is copy-pasted between the two files, and the scrollIntoView polyfill's if (!Element.prototype.scrollIntoView) guard means it's assigned only once as a plain vi.fn(). In Vitest 4, vi.restoreAllMocks() only restores spies created with vi.spyOn, so this mock's call history is never reset between tests — a latent isolation gap for any future test that asserts on scrollIntoView calls.

  • tests/setup/jsdom.setup.ts#L12-L40: Export installMatchMediaStub for reuse, and replace the guarded assignment with vi.spyOn(Element.prototype, "scrollIntoView").mockImplementation(() => {}) (or an explicit mockClear()) in beforeEach so it resets consistently every test.
  • tests/accessible-table.dom.test.tsx#L11-L26: Import and reuse the shared stub from tests/setup/jsdom.setup.ts instead of re-declaring setMatchMedia locally.
♻️ Proposed fix
--- a/tests/setup/jsdom.setup.ts
+++ b/tests/setup/jsdom.setup.ts
@@
-function installMatchMediaStub(matches = false) {
+export function installMatchMediaStub(matches = false) {
   Object.defineProperty(window, "matchMedia", {
@@
 beforeEach(() => {
   installMatchMediaStub(false);
-  // jsdom does not implement scrollIntoView; components call it on focus/expand.
-  if (!Element.prototype.scrollIntoView) {
-    Element.prototype.scrollIntoView = vi.fn();
-  }
+  // jsdom does not implement scrollIntoView; components call it on focus/expand.
+  // Re-spy every test so history/implementation resets under Vitest 4's
+  // restoreAllMocks (which only restores vi.spyOn spies, not plain vi.fn()s).
+  if (!vi.isMockFunction(Element.prototype.scrollIntoView)) {
+    Element.prototype.scrollIntoView = () => {};
+  }
+  vi.spyOn(Element.prototype, "scrollIntoView").mockImplementation(() => {});
 });
--- a/tests/accessible-table.dom.test.tsx
+++ b/tests/accessible-table.dom.test.tsx
@@
-import { render, screen } from "`@testing-library/react`";
-import userEvent from "`@testing-library/user-event`";
-import { describe, expect, it, vi } from "vitest";
-import { AccessibleTable } from "`@/components/AccessibleTable`";
-
-function setMatchMedia(matches: boolean) {
-  Object.defineProperty(window, "matchMedia", {
-    writable: true,
-    configurable: true,
-    value: vi.fn().mockImplementation((query: string) => ({
-      matches,
-      media: query,
-      onchange: null,
-      addEventListener: vi.fn(),
-      removeEventListener: vi.fn(),
-      addListener: vi.fn(),
-      removeListener: vi.fn(),
-      dispatchEvent: vi.fn(),
-    })),
-  });
-}
+import { render, screen } from "`@testing-library/react`";
+import userEvent from "`@testing-library/user-event`";
+import { describe, expect, it } from "vitest";
+import { AccessibleTable } from "`@/components/AccessibleTable`";
+import { installMatchMediaStub as setMatchMedia } from "./setup/jsdom.setup";

Based on learnings, this relies on the coding guideline for **/*.{js,jsx,ts,tsx,mjs,cjs}: "Automatic reviews may emit at most three inline findings, and inline comments are limited to P0/P1 issues; P2 context belongs in one summary."

🤖 Prompt for 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.

In `@tests/setup/jsdom.setup.ts` around lines 12 - 40, Deduplicate the browser
stubs by exporting installMatchMediaStub from tests/setup/jsdom.setup.ts and
updating its beforeEach setup to spy on Element.prototype.scrollIntoView with a
resettable mock on every test instead of the one-time guarded assignment. In
tests/setup/jsdom.setup.ts#L12-L40, make the shared stub and per-test
scrollIntoView reset change; in tests/accessible-table.dom.test.tsx#L11-L26,
remove the local setMatchMedia declaration and import and reuse
installMatchMediaStub, with no other behavior changes.
🤖 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 @.github/workflows/ci.yml:
- Around line 197-203: Resolve the conflict in the bundle-size CI step by
removing all conflict markers and retaining the intended bundle budget checks,
including both check:bundle-size and check:bundle-budget if both are required.
Confirm and preserve their intended gating or warn-only behavior so the
resulting workflow is valid YAML.

In @.husky/pre-commit:
- Line 1: Update the pre-commit hook command to invoke the project’s locally
installed lint-staged binary directly instead of using npx. Preserve the
existing lint-staged arguments and ensure the hook fails when the local
dependency is unavailable.

In @.lintstagedrc.json:
- Around line 2-3: Update the second formatting glob in the lint-staged
configuration to include scss alongside css, ensuring staged SCSS files are
processed by Prettier while preserving the existing JSON, Markdown, YAML, and
CSS coverage.

In `@src/app/api/monitoring/route.ts`:
- Around line 61-65: Update the upstream Sentry relay fetch in the monitoring
route to enforce a finite timeout, using an AbortSignal or equivalent request
cancellation mechanism and the existing timeout conventions if available. Ensure
the request is aborted when the timeout expires while preserving the current
POST payload and headers.

In `@src/instrumentation.ts`:
- Around line 1-34: Resolve all remaining merge conflicts by removing every
conflict marker and retaining the intended combined implementation. In
src/instrumentation.ts lines 1-34 and 91-119, merge the Sentry initialization
with the Next.js instrumentation flow; cleanly merge the conflicting environment
definitions in src/lib/env.ts lines 19-39; and resolve both
dependency/configuration conflicts in package.json lines 13-21 and 148-152,
ensuring each file remains valid and builds successfully.

In `@src/lib/observability/sentry-scrub.ts`:
- Around line 1-69: Update scrubSentryEvent to remove event.breadcrumbs along
with the other high-risk request and identity fields, ensuring client-side
console, fetch, and XHR breadcrumb data cannot be sent to Sentry. Do not alter
the existing redaction and truncation behavior.

In `@tests/ui-accessibility.spec.ts`:
- Around line 2-6: Resolve all remaining merge conflicts in
tests/ui-accessibility.spec.ts: reconcile the Playwright import to include the
required types, retain a single expectNoBlockingAxeViolations implementation,
and update every caller in lines 78-148 to pass the TestInfo value required by
that helper’s attachment contract.

---

Nitpick comments:
In `@tests/setup/jsdom.setup.ts`:
- Around line 12-40: Deduplicate the browser stubs by exporting
installMatchMediaStub from tests/setup/jsdom.setup.ts and updating its
beforeEach setup to spy on Element.prototype.scrollIntoView with a resettable
mock on every test instead of the one-time guarded assignment. In
tests/setup/jsdom.setup.ts#L12-L40, make the shared stub and per-test
scrollIntoView reset change; in tests/accessible-table.dom.test.tsx#L11-L26,
remove the local setMatchMedia declaration and import and reuse
installMatchMediaStub, with no other behavior changes.
🪄 Autofix (Beta)

✅ Autofix completed


ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: f4c31514-083e-4010-b071-daafdfcad7c2

📥 Commits

Reviewing files that changed from the base of the PR and between ab09a8d and 672fffc.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (26)
  • .editorconfig
  • .github/CODEOWNERS
  • .github/workflows/ci.yml
  • .husky/pre-commit
  • .lintstagedrc.json
  • .size-limit.json
  • docs/site-map.md
  • knip.json
  • next.config.ts
  • package.json
  • scripts/check-client-bundle-secrets.mjs
  • src/app/api/monitoring/route.ts
  • src/app/global-error.tsx
  • src/components/route-error-boundary.tsx
  • src/instrumentation-client.ts
  • src/instrumentation.ts
  • src/lib/env.ts
  • src/lib/logger.ts
  • src/lib/observability/sentry-client.ts
  • src/lib/observability/sentry-scrub.ts
  • tests/accessible-table.dom.test.tsx
  • tests/sentry-observability.test.ts
  • tests/setup/jsdom.setup.ts
  • tests/ui-accessibility.spec.ts
  • tests/use-event-callback.dom.test.tsx
  • vitest.config.mts

Comment thread .github/workflows/ci.yml Outdated
Comment on lines +197 to +203
<<<<<<< HEAD
- name: Client bundle size budget
run: npm run check:bundle-size
=======
- name: Bundle budget (warn-only until a baseline is enforced)
run: npm run check:bundle-budget
>>>>>>> origin/main

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.

🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win

Resolve the merge conflict before merging.

The literal conflict markers make .github/workflows/ci.yml invalid YAML, so the CI workflow cannot load. Remove the markers and retain the intended bundle-budget steps—likely both checks—after confirming their desired gating behavior.

Proposed fix
-<<<<<<< HEAD
       - name: Client bundle size budget
         run: npm run check:bundle-size
-=======
       - name: Bundle budget (warn-only until a baseline is enforced)
         run: npm run check:bundle-budget
->>>>>>> origin/main
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<<<<<<< HEAD
- name: Client bundle size budget
run: npm run check:bundle-size
=======
- name: Bundle budget (warn-only until a baseline is enforced)
run: npm run check:bundle-budget
>>>>>>> origin/main
- name: Client bundle size budget
run: npm run check:bundle-size
- name: Bundle budget (warn-only until a baseline is enforced)
run: npm run check:bundle-budget
🧰 Tools
🪛 actionlint (1.7.12)

[error] 197-197: could not parse as YAML: could not find expected ':'

(syntax-check)

🪛 YAMLlint (1.37.1)

[error] 198-198: syntax error: could not find expected ':'

(syntax)

🤖 Prompt for 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.

In @.github/workflows/ci.yml around lines 197 - 203, Resolve the conflict in the
bundle-size CI step by removing all conflict markers and retaining the intended
bundle budget checks, including both check:bundle-size and check:bundle-budget
if both are required. Confirm and preserve their intended gating or warn-only
behavior so the resulting workflow is valid YAML.

Source: Linters/SAST tools

Comment thread .husky/pre-commit Outdated
@@ -0,0 +1 @@
npx lint-staged

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.

🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n[.husky/pre-commit]\n'
cat -n .husky/pre-commit

printf '\n[package.json packageManager/deps]\n'
python3 - <<'PY'
import json
from pathlib import Path
p = Path('package.json')
if p.exists():
    data = json.loads(p.read_text())
    for k in ('packageManager', 'devDependencies', 'dependencies'):
        if k in data:
            print(f'\n{k}:')
            v = data[k]
            if isinstance(v, dict):
                for name in sorted(v):
                    if name == 'lint-staged' or k == 'packageManager':
                        print(f'  {name}: {v[name]}')
            else:
                print(v)
else:
    print('package.json not found')
PY

printf '\n[npx help/version]\n'
npx --version || true
npm --version || true

Repository: BigSimmo/Database

Length of output: 879


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n[repo root files of interest]\n'
git ls-files .husky/pre-commit package.json pnpm-lock.yaml package-lock.json yarn.lock npm-shrinkwrap.json .npmrc .node-version .nvmrc | cat

printf '\n[package.json excerpt]\n'
cat -n package.json | sed -n '1,120p'

printf '\n[husky hook]\n'
cat -n .husky/pre-commit

Repository: BigSimmo/Database

Length of output: 10682


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n[npm/npx help]\n'
npm exec --help | sed -n '1,120p' || true
printf '\n'
npx --help | sed -n '1,120p' || true

printf '\n[lint-staged in lockfile/package]\n'
rg -n '"lint-staged"|lint-staged' package.json package-lock.json | sed -n '1,120p'

Repository: BigSimmo/Database

Length of output: 2867


🌐 Web query:

npm 11 npx --no-install deprecated alias npm exec local remote package install fallback documentation

💡 Result:

In npm 11, the --no-install option is deprecated and will be converted to --no [1][2]. The npx binary itself is a wrapper that uses the npm exec command, and this deprecation is part of the ongoing evolution of npx functionality within the npm CLI [1][3]. Key details regarding this deprecation and related functionality include: 1. --no-install Deprecation: This option is deprecated and will be converted to --no [1][4]. This change aligns with the broader removal of older, less advisable npx features [1]. 2. Shell Fallback: Shell fallback functionality has been removed, as it is considered insecure and not advisable [1][2]. 3. npx vs npm exec: The npx binary is now maintained as part of the npm CLI [1][5]. While npx provides some backward compatibility, npm exec is the primary command for executing local or remote packages [3][4]. When using npm exec, the use of a double-hyphen (--) is recommended to explicitly stop npm from parsing command-line options and ensure they are passed to the executed command instead [3][4]. 4. Package Installation: npm exec (and the npx wrapper) will attempt to install missing packages by default to execute them, unless configured otherwise or when restricted by specific CLI options [1][3]. The --no-install flag (now deprecated) was previously used to prevent this behavior [5][4]. For current and future use, it is recommended to use the npm exec command directly or the npx binary as intended by modern npm versions, while transitioning away from deprecated flags like --no-install [1][3].

Citations:


🌐 Web query:

npm exec --no npx --no-install docs npm 11 prevent install missing package

💡 Result:

In npm 11, the --no-install option is deprecated and converted to --no [1][2]. This flag does not explicitly exist to "prevent install of a missing package" in the way that it disables the download feature; rather, npm exec is designed to execute packages that are either locally available or fetched remotely [2][3]. If you are attempting to prevent npm from automatically downloading a package that is not present in your local dependencies, there is no direct --no-install flag that serves this purpose [1][2]. Key behaviors to note: 1. Deprecation: The --no-install option has been deprecated for several versions and is automatically converted to --no [4][5][6]. 2. Execution Context: npm exec will attempt to use locally installed packages first. If the requested package is not found locally, it will attempt to fetch it remotely [2][3]. 3. Prompts: To prevent unexpected installations, npm provides prompts before installing missing packages. You can suppress these prompts using --yes or --no, but these flags control the interactive prompt itself rather than disabling the installation mechanism entirely [4][5][6]. If you want to ensure that only locally available binaries are executed, you should ensure the dependencies are already present in your project, or check that your environment constraints (such as running in a non-TTY environment or a CI setting) influence how npm handles missing packages [5][2]. In CI environments or when standard input is not a TTY, npm often defaults to --yes (auto-installing) [5]. To avoid unintended remote fetches, you should rely on local installation via npm install prior to execution [2].

Citations:


Avoid npx in the hook. .husky/pre-commit:1 should call the local lint-staged binary directly so the hook fails closed if dependencies are missing; npx can still resolve a remote package, and --no-install is deprecated in npm 11.

🤖 Prompt for 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.

In @.husky/pre-commit at line 1, Update the pre-commit hook command to invoke
the project’s locally installed lint-staged binary directly instead of using
npx. Preserve the existing lint-staged arguments and ensure the hook fails when
the local dependency is unavailable.

Comment thread .lintstagedrc.json Outdated
Comment on lines +2 to +3
"*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}": ["prettier --write --ignore-unknown", "eslint --fix"],
"*.{json,md,yml,yaml,css}": ["prettier --write --ignore-unknown"]

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.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Include SCSS in the formatting rule.

The staged-file formatter covers CSS but not SCSS, so SCSS changes can bypass the pre-commit formatting check. Add scss to the second glob.

As per coding guidelines, the formatter scope must cover **/*.{js,jsx,ts,tsx,css,scss}.

Suggested fix
-  "*.{json,md,yml,yaml,css}": ["prettier --write --ignore-unknown"]
+  "*.{json,md,yml,yaml,css,scss}": ["prettier --write --ignore-unknown"]
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
"*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}": ["prettier --write --ignore-unknown", "eslint --fix"],
"*.{json,md,yml,yaml,css}": ["prettier --write --ignore-unknown"]
"*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}": ["prettier --write --ignore-unknown", "eslint --fix"],
"*.{json,md,yml,yaml,css,scss}": ["prettier --write --ignore-unknown"]
🤖 Prompt for 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.

In @.lintstagedrc.json around lines 2 - 3, Update the second formatting glob in
the lint-staged configuration to include scss alongside css, ensuring staged
SCSS files are processed by Prettier while preserving the existing JSON,
Markdown, YAML, and CSS coverage.

Source: Coding guidelines

Comment on lines +61 to +65
const upstream = await fetch(`https://${expected.host}/api/${expected.projectId}/envelope/`, {
method: "POST",
body: envelope,
headers: { "Content-Type": "application/x-sentry-envelope" },
});

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.

🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win

Add a timeout to the upstream Sentry relay fetch.

This fetch to Sentry's ingest host has no timeout. If the upstream host is slow or unreachable, this request-thread call can hang indefinitely, tying up server capacity on every client error report relayed through this tunnel.

⏱️ Proposed fix: bound the upstream request with a timeout
   const upstream = await fetch(`https://${expected.host}/api/${expected.projectId}/envelope/`, {
     method: "POST",
     body: envelope,
     headers: { "Content-Type": "application/x-sentry-envelope" },
+    signal: AbortSignal.timeout(5000),
   });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const upstream = await fetch(`https://${expected.host}/api/${expected.projectId}/envelope/`, {
method: "POST",
body: envelope,
headers: { "Content-Type": "application/x-sentry-envelope" },
});
const upstream = await fetch(`https://${expected.host}/api/${expected.projectId}/envelope/`, {
method: "POST",
body: envelope,
headers: { "Content-Type": "application/x-sentry-envelope" },
signal: AbortSignal.timeout(5000),
});
🤖 Prompt for 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.

In `@src/app/api/monitoring/route.ts` around lines 61 - 65, Update the upstream
Sentry relay fetch in the monitoring route to enforce a finite timeout, using an
AbortSignal or equivalent request cancellation mechanism and the existing
timeout conventions if available. Ensure the request is aborted when the timeout
expires while preserving the current POST payload and headers.

Comment thread src/instrumentation.ts Outdated
Comment on lines +1 to +34
<<<<<<< HEAD
// Gated server-side Sentry init. SENTRY_DSN is unset by default, so this is a
// no-op (and never imports the SDK) until Sentry is configured. Runs for both the
// Node.js and Edge runtimes so server + proxy errors are captured. The SDK is
// dynamically imported so a deployment without a DSN never loads it.
async function initSentryServer() {
const dsn = process.env.SENTRY_DSN;
if (!dsn) return;
const [Sentry, { scrubSentryErrorEvent }] = await Promise.all([
import("@sentry/nextjs"),
import("@/lib/observability/sentry-scrub"),
]);
Sentry.init({
dsn,
environment: process.env.SENTRY_ENVIRONMENT || process.env.NODE_ENV,
tracesSampleRate: Number(process.env.SENTRY_TRACES_SAMPLE_RATE ?? 0) || 0,
sendDefaultPii: false,
beforeSend: scrubSentryErrorEvent,
});
}
=======
import type { Instrumentation } from "next";
>>>>>>> origin/main

// Next.js calls register() once when a server instance starts, before it serves
// any requests. We use it to fail fast: a clinical production server must be fully
// and correctly configured rather than silently degrading — or, worse, serving
// unauthenticated demo content — on the first request. See production-readiness
// plan items 0.1 and 0.3.
export async function register() {
// Wire error tracking first so a misconfiguration thrown by the gates below is
// itself reportable. No-op unless SENTRY_DSN is set.
await initSentryServer();

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.

🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

set -euo pipefail

printf '\n== conflict markers ==\n'
rg -n '^(<<<<<<<|=======|>>>>>>>)' src/instrumentation.ts src/lib/env.ts package.json || true

printf '\n== instrumentation excerpt ==\n'
cat -n src/instrumentation.ts | sed -n '1,160p'

printf '\n== env excerpt ==\n'
cat -n src/lib/env.ts | sed -n '1,80p'

printf '\n== package.json scripts/deps excerpts ==\n'
cat -n package.json | sed -n '1,40p'
printf '\n-- deps slice --\n'
cat -n package.json | sed -n '140,170p'

Repository: BigSimmo/Database

Length of output: 16769


Resolve the merge conflicts in src/instrumentation.ts, src/lib/env.ts, and package.json Literal <<<<<<< / ======= / >>>>>>> markers are still present, so the branch won’t parse or build until these files are merged cleanly.

🧰 Tools
🪛 Biome (2.5.3)

[error] 1-6: Expected a statement but instead found '<<<<<<< HEAD
// Gated server-side Sentry init. SENTRY_DSN is unset by default, so this is a
// no-op (and never imports the SDK) until Sentry is configured. Runs for both the
// Node.js and Edge runtimes so server + proxy errors are captured. The SDK is
// dynamically imported so a deployment without a DSN never loads it.
async'.

(parse)


[error] 9-9: await is only allowed within async functions and at the top levels of modules.

(parse)


[error] 21-21: Expected a statement but instead found '======='.

(parse)


[error] 23-23: Expected a statement but instead found '>>>>>>> origin/main'.

(parse)

📍 Affects 3 files
  • src/instrumentation.ts#L1-L34 (this comment)
  • src/instrumentation.ts#L91-L119
  • src/lib/env.ts#L19-L39
  • package.json#L13-L21
  • package.json#L148-L152
🤖 Prompt for 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.

In `@src/instrumentation.ts` around lines 1 - 34, Resolve all remaining merge
conflicts by removing every conflict marker and retaining the intended combined
implementation. In src/instrumentation.ts lines 1-34 and 91-119, merge the
Sentry initialization with the Next.js instrumentation flow; cleanly merge the
conflicting environment definitions in src/lib/env.ts lines 19-39; and resolve
both dependency/configuration conflicts in package.json lines 13-21 and 148-152,
ensuring each file remains valid and builds successfully.

Comment on lines +1 to +69
// Privacy scrubber for Sentry events (client + server). This app handles clinical
// queries/answers and user emails; NONE of that may leave the app in an error
// report. The scrubber runs as Sentry's `beforeSend`/`beforeSendTransaction` hook
// and enforces the same PII contract the structured logger uses (SENSITIVE_KEY,
// imported from src/lib/logger.ts as the single source of truth):
// - strips request cookies, headers, query string and body outright;
// - drops user identifiers;
// - redacts any extra/context/tag value whose KEY looks sensitive;
// - truncates long strings so a stray query embedded in an error message is bounded.
// It is intentionally conservative: over-redaction only loses debugging detail,
// under-redaction leaks patient data.
import type { ErrorEvent, Event } from "@sentry/nextjs";
import { SENSITIVE_KEY } from "@/lib/logger";

const REDACTED = "[redacted]";
const MAX_STRING_LENGTH = 512;
const MAX_DEPTH = 6;

function redactByKey(value: unknown, depth = 0): unknown {
if (value === null || value === undefined) return value;
if (typeof value === "string") {
return value.length > MAX_STRING_LENGTH ? `${value.slice(0, MAX_STRING_LENGTH)}…[truncated]` : value;
}
if (typeof value === "number" || typeof value === "boolean") return value;
if (depth >= MAX_DEPTH) return "[depth-limited]";
if (Array.isArray(value)) return value.slice(0, 50).map((item) => redactByKey(item, depth + 1));
if (typeof value === "object") {
const out: Record<string, unknown> = {};
for (const [key, val] of Object.entries(value as Record<string, unknown>)) {
out[key] = SENSITIVE_KEY.test(key) ? REDACTED : redactByKey(val, depth + 1);
}
return out;
}
return String(value);
}

function truncate(value: string | undefined): string | undefined {
if (typeof value !== "string") return value;
return value.length > MAX_STRING_LENGTH ? `${value.slice(0, MAX_STRING_LENGTH)}…[truncated]` : value;
}

export function scrubSentryEvent<T extends Event>(event: T): T {
// Request context is the highest-risk surface: cookies/auth headers are secrets,
// and query strings / bodies can carry the clinical query. Remove them wholesale.
if (event.request) {
delete event.request.cookies;
delete event.request.query_string;
delete event.request.data;
if (event.request.headers) {
event.request.headers = redactByKey(event.request.headers) as Record<string, string>;
}
}

// No user identity (email/id/ip) — the request-hash correlation id is enough.
delete event.user;

if (event.extra) event.extra = redactByKey(event.extra) as Record<string, unknown>;
if (event.contexts) event.contexts = redactByKey(event.contexts) as typeof event.contexts;
if (event.tags) event.tags = redactByKey(event.tags) as typeof event.tags;

// Defensively bound exception/message text so a query echoed into an Error
// message cannot arrive verbatim.
if (event.message) event.message = truncate(event.message) as string;
for (const exception of event.exception?.values ?? []) {
if (exception.value) exception.value = truncate(exception.value);
}

return event;
}

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.

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== Files related to Sentry ==\n'
git ls-files | rg '(^|/)(sentry|instrumentation).*\\.(ts|tsx|js|jsx|mjs|cjs)$|sentry.*\\.(ts|tsx|js|jsx|mjs|cjs)$'

printf '\n== Search for breadcrumbs / beforeBreadcrumb / beforeSend ==\n'
rg -n --hidden --glob '!node_modules' --glob '!dist' --glob '!build' \
  'beforeBreadcrumb|breadcrumbs|beforeSendTransaction|beforeSend\\s*:' src .

printf '\n== Map candidate files ==\n'
for f in src/instrumentation.ts src/instrumentation-client.ts src/lib/observability/sentry-scrub.ts; do
  if [ -f "$f" ]; then
    echo "--- $f"
    ast-grep outline "$f" --view expanded || true
  fi
done

Repository: BigSimmo/Database

Length of output: 186


🏁 Script executed:

#!/bin/bash
set -euo pipefail

for f in src/instrumentation.ts src/instrumentation-client.ts src/lib/observability/sentry-scrub.ts; do
  if [ -f "$f" ]; then
    echo "===== $f ====="
    nl -ba "$f" | sed -n '1,240p'
    echo
  fi
done

Repository: BigSimmo/Database

Length of output: 230


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '== Relevant tracked files ==\n'
git ls-files | rg '(^|/)(instrumentation|sentry).*\\.(ts|tsx|js|jsx|mjs|cjs)$|sentry-scrub\\.ts$' || true

printf '\n== Candidate paths in src ==\n'
fd -a 'instrumentation.*|sentry-scrub\\.ts' src 2>/dev/null || true

printf '\n== Read candidate files if present ==\n'
for f in src/instrumentation.ts src/instrumentation-client.ts src/lib/observability/sentry-scrub.ts; do
  if [ -f "$f" ]; then
    echo "--- $f"
    sed -n '1,220p' "$f" | cat -n
  fi
done

Repository: BigSimmo/Database

Length of output: 13724


🏁 Script executed:

#!/bin/bash
set -euo pipefail

for f in src/lib/observability/sentry-client.ts src/lib/observability/error-capture.ts src/lib/observability/sentry-scrub.ts src/instrumentation-client.ts; do
  if [ -f "$f" ]; then
    echo "===== $f ====="
    sed -n '1,240p' "$f" | cat -n
    echo
  fi
done

printf '\n== Breadcrumb-related search in observability ==\n'
rg -n 'beforeBreadcrumb|breadcrumbs|breadcrumb' src/lib/observability src/instrumentation-client.ts src/instrumentation.ts

Repository: BigSimmo/Database

Length of output: 11449


Drop breadcrumbs in the client Sentry path src/lib/observability/sentry-scrub.ts still leaves event.breadcrumbs intact, so the browser init can ship console/fetch/XHR breadcrumbs with clinical query text or URLs. Strip breadcrumbs here, or disable them in src/instrumentation-client.ts as well.

🤖 Prompt for 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.

In `@src/lib/observability/sentry-scrub.ts` around lines 1 - 69, Update
scrubSentryEvent to remove event.breadcrumbs along with the other high-risk
request and identity fields, ensuring client-side console, fetch, and XHR
breadcrumb data cannot be sent to Sentry. Do not alter the existing redaction
and truncation behavior.

Comment thread tests/ui-accessibility.spec.ts Outdated
@coderabbitai

coderabbitai Bot commented Jul 14, 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.

Fixes Applied Successfully

Fixed 4 file(s) based on 6 unresolved review comments.

Files modified:

  • .lintstagedrc.json
  • package-lock.json
  • src/app/api/monitoring/route.ts
  • src/lib/observability/sentry-scrub.ts

Commit: 6bc52ff77455bfeeb4b8b4f4228e9e23bb2bd3b1

The changes have been pushed to the claude/repo-improvement-review-09945c branch.

Time taken: 7m 49s

@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 @.githooks/pre-commit:
- Line 24: Update the lint-staged invocation in the pre-commit hook to use npx
--no (or npm exec --no), ensuring it only runs the locally installed lint-staged
binary and fails when unavailable instead of installing from the registry.

In `@src/lib/env.ts`:
- Line 31: Update the NEXT_PUBLIC_SENTRY_DSN schema in envSchema to normalize an
empty string to undefined before URL validation, matching the existing
SENTRY_DSN behavior. Preserve valid URLs and the optional unset behavior so
envSchema.parse(process.env) accepts blank public DSNs.
🪄 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: d1e1ad16-6a57-467b-a586-576ae3029b47

📥 Commits

Reviewing files that changed from the base of the PR and between 672fffc and 06db701.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (4)
  • .githooks/pre-commit
  • .githooks/pre-push
  • package.json
  • src/lib/env.ts
💤 Files with no reviewable changes (1)
  • package.json

Comment thread .githooks/pre-commit
exit 0
fi

npx lint-staged

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.

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '--- .githooks/pre-commit ---\n'
cat -n .githooks/pre-commit

printf '\n--- package.json ---\n'
cat -n package.json

printf '\n--- lockfiles / package manager hints ---\n'
git ls-files | rg '(^|/)(package-lock\.json|pnpm-lock\.yaml|yarn\.lock|bun\.lockb|npm-shrinkwrap\.json)$|(^|/)\.npmrc$|(^|/)\.yarnrc(\.yml)?$'

Repository: BigSimmo/Database

Length of output: 246


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- .githooks/pre-commit ---'
cat -n .githooks/pre-commit

echo
echo '--- package.json ---'
cat -n package.json

echo
echo '--- lockfiles / package manager hints ---'
git ls-files | rg '(^|/)(package-lock\.json|pnpm-lock\.yaml|yarn\.lock|bun\.lockb|npm-shrinkwrap\.json)$|(^|/)\.npmrc$|(^|/)\.yarnrc(\.yml)?$'

Repository: BigSimmo/Database

Length of output: 15456


🌐 Web query:

npm 11 npx --no-install npm exec --no-install local binary missing install from registry behavior

💡 Result:

In npm 11 (as well as all versions since npm v7), the behavior of npx and npm exec has been standardized around the npm exec command [1][2][3]. Regarding your specific query: 1. Deprecation of --no-install: The --no-install option is deprecated and is automatically converted to --no [1][2][4]. 2. Behavior with --no: When you use --no (or the deprecated --no-install), npm is explicitly instructed not to attempt to install any missing packages from the registry [2][3][5]. 3. Handling of Local Binaries: - By design, npx and npm exec include locally installed package executables in the PATH of the executed command [1][6][7]. - If a binary is already present in your project's local dependencies (e.g., in node_modules/.bin), it should be found and executed correctly regardless of the --no flag [1][3]. - If the binary is missing from the local dependencies and you use --no, npm exec will fail because it is prohibited from installing the package from the registry to resolve the command [2][5][8]. If you find that a locally available binary is not being detected when using --no, it may be related to specific configuration issues, such as workspace contexts or how the binary is defined in package.json [9]. If the binary is not part of the project's dependency tree, npm cannot execute it without performing an installation, which the --no flag prevents [2][3].

Citations:


Prevent npx from installing lint-staged here. Use npx --no lint-staged (or npm exec --no lint-staged) so a missing local binary fails instead of fetching an unpinned package from the registry.

🤖 Prompt for 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.

In @.githooks/pre-commit at line 24, Update the lint-staged invocation in the
pre-commit hook to use npx --no (or npm exec --no), ensuring it only runs the
locally installed lint-staged binary and fails when unavailable instead of
installing from the registry.

Comment thread src/lib/env.ts
// tree-shaken out of the client bundle at build time via __SENTRY_ENABLED__
// DefinePlugin literal. The public DSN is safe to expose; keep SENTRY_AUTH_TOKEN
// (source-map upload) out of any NEXT_PUBLIC_* var — it is build-time only.
NEXT_PUBLIC_SENTRY_DSN: z.string().url().optional(),

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.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '== file list ==\n'
git ls-files src/lib/env.ts

printf '\n== relevant references ==\n'
rg -n "NEXT_PUBLIC_SENTRY_DSN|SENTRY_DSN|preprocess|blank|string\\.url\\(\\)\\.optional\\(\\)" src/lib/env.ts src -g '!**/node_modules/**' || true

printf '\n== env.ts excerpt ==\n'
nl -ba src/lib/env.ts | sed -n '1,120p'

Repository: BigSimmo/Database

Length of output: 6098


🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
from pathlib import Path
p = Path('src/lib/env.ts')
text = p.read_text()
print('contains NEXT_PUBLIC_SENTRY_DSN:', 'NEXT_PUBLIC_SENTRY_DSN' in text)
print('contains SENTRY_DSN blank normalization:', 'trim() === ""' in text or 'empty strings' in text)
PY

Repository: BigSimmo/Database

Length of output: 239


🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
import re, pathlib
text = pathlib.Path('src/lib/env.ts').read_text()
# show the exact schema lines around SENTRY fields
for m in re.finditer(r'SENTRY_DSN|NEXT_PUBLIC_SENTRY_DSN', text):
    start = max(0, m.start()-180)
    end = min(len(text), m.end()+220)
    print('---')
    print(text[start:end])
PY

Repository: BigSimmo/Database

Length of output: 997


🏁 Script executed:

#!/bin/bash
set -euo pipefail

nl -ba src/lib/env.ts | sed -n '1,120p'

Repository: BigSimmo/Database

Length of output: 195


Treat an empty public DSN as unset.
NEXT_PUBLIC_SENTRY_DSN="" is still rejected by z.string().url().optional(), so optional client-side Sentry config can abort envSchema.parse(process.env) during startup. Normalize blank strings to undefined here, matching SENTRY_DSN.

🤖 Prompt for 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.

In `@src/lib/env.ts` at line 31, Update the NEXT_PUBLIC_SENTRY_DSN schema in
envSchema to normalize an empty string to undefined before URL validation,
matching the existing SENTRY_DSN behavior. Preserve valid URLs and the optional
unset behavior so envSchema.parse(process.env) accepts blank public DSNs.

Fixed 4 file(s) based on 6 unresolved review comments.

Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
@BigSimmo

Copy link
Copy Markdown
Owner Author

Superseded by #636, which is now merged into main (commit fa850f3).

This branch was 40 commits stale and its tooling overlapped work main shipped independently (git-hooks, bundle-budget, axe, server-side @sentry/node). Rather than land competing/duplicate systems, the clean reconciliation in #636 shipped only the four non-overlapping tools (.editorconfig, CODEOWNERS, jsdom + Testing Library, knip) off current main.

The outstanding merge conflicts and review comments here all target code that was intentionally dropped and never merged. Closing to avoid resurrecting the duplicate approach. The one valid nitpick that applies to merged code (jsdom scrollIntoView stub reset) is being handled in a small follow-up PR.

@BigSimmo BigSimmo closed this Jul 14, 2026
auto-merge was automatically disabled July 14, 2026 05:56

Pull request was closed

@BigSimmo
BigSimmo deleted the claude/repo-improvement-review-09945c branch July 14, 2026 07:00
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