Skip to content

Enforce answer summaryMode contract, scan composite GitHub actions, accessibility tweaks, and docs/CI hygiene updates - #1530

Merged
BigSimmo merged 3 commits into
mainfrom
codex/perform-comprehensive-code-review
Jul 31, 2026
Merged

Enforce answer summaryMode contract, scan composite GitHub actions, accessibility tweaks, and docs/CI hygiene updates#1530
BigSimmo merged 3 commits into
mainfrom
codex/perform-comprehensive-code-review

Conversation

@BigSimmo

Copy link
Copy Markdown
Owner

Motivation

  • Prevent silent contract drift where non-stream /api/answer accepted summaryMode but ran full answer generation, and tighten summary-mode scope validation to avoid summarizing a different document than was validated.
  • Harden CI supply-chain checks by scanning local composite actions and workflow files for unpinned uses: references and pinning policy gaps, and extend PR policy coverage to release branches.
  • Improve UI accessibility by using native disabled for unavailable controls and remove interactive-but-href-less anchors in mockups.
  • Remove a noisy/unrecognized npm config and update various repo audit and process docs to record findings and remediation guidance.

Description

  • Add release/** to the PR policy workflow trigger in .github/workflows/pr-policy.yml so release-targeted PRs run the same metadata checks as main PRs.
  • Reject summaryMode on the non-stream answer route by returning a summary_mode_stream_required 400 error in src/app/api/answer/route.ts, and scope streamed summary requests to the exact documentId in src/app/api/answer/stream/route.ts.
  • Strengthen request validation in src/lib/validation/answer-request.ts to reject conflicting or multi-document summaryMode payloads and add tests/answer-summary-mode-contract.test.ts to assert the contract and route-level rejection.
  • Extend the GitHub Actions pin checker (scripts/check-github-action-pins.mjs) to discover and scan .github/actions/**/action.yml composite actions, add a self-test harness, and improve workflow file discovery/collection logic.
  • Update UI components to prefer native disabled over aria-disabled and non-actionable anchors to non-interactive elements, including FavouritesHub, differential density controls, and mockups; and update the corresponding DOM tests under tests/ to assert the new behavior.
  • Remove the unknown allow-scripts=true entry from .npmrc and add/modify a number of audit and process docs (docs/*) to reflect findings and remediation steps from the repo-wide sweep.

Testing

  • Added unit/dom tests: tests/answer-summary-mode-contract.test.ts and updated tests/favourites-hub-unavailable-controls.dom.test.tsx, and the test suite asserts request-schema rules and UI disabled semantics; these tests pass locally under the repo test run.
  • Static checks executed include typecheck (tsc --noEmit) which passed, and the test suite (npm run test) which passed in the verification run reported (703 passed, 2 skipped); check:edge:functions also passed in that run.
  • Linting could not be executed in the authoring environment due to missing node_modules/eslint (pre-existing local environment gap), and npm/format gates are noted in docs as requiring npm ci / Node 24 to reproduce fully.

Codex Task

@coderabbitai

coderabbitaiBot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

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

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

Next review available in:55 minutes

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

How can I continue?

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

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

How do review limits work?

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

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

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 5586ae30-6a59-4c8b-8fee-37df9b514850

📥 Commits

Reviewing files that changed from the base of the PR and between ed849c7 and bebc76d.

📒 Files selected for processing (3)
  • src/components/differentials/differential-presentation-workflow-page.tsx
  • tests/answer-summary-mode-contract.test.ts
  • tests/mobile-interaction-regressions.test.ts

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

@ecc-tools

ecc-toolsBot commented Jul 31, 2026

Copy link
Copy Markdown

🔒 Upgrade Required

Private repository analysis requires Pro or Enterprise.

Upgrade: https://ecc.tools/pricing?plan=pro


ECC Tools keeps the core app open, and puts private repos, team features, and enterprise controls behind paid tiers.

@supabase

supabaseBot commented Jul 31, 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 ↗︎.

@BigSimmo
BigSimmo marked this pull request as ready for review July 31, 2026 11:21
@BigSimmo

Copy link
Copy Markdown
OwnerAuthor

@cursoragent Work the current open PR end-to-end. Confirm the PR number and GitHub head first from context. If more than one open PR could apply, stop and say which one you would use and why.

Fetch and start from the remote tip that matches that GitHub head. If the named branch ref is missing or stale, use the PR head ref. Preserve unrelated local WIP, including any local-only ledger commits; do not discard dirty work, and do not treat a local-ahead commit as the reviewed tip. Do not merge the PR, force-push, rebase, or arm auto-merge unless I explicitly ask. No provider-backed gates without separate approval. If you cannot push or resolve threads, diagnose and comment only; if inline replies fail, resolve when possible and put dispositions in the summary comment. If auto-merge is already armed, push only for a real blocker, and avoid pushes that would cancel in-flight required CI unless the push itself clears that blocker.

If the PR is already merged or closed: confirm the head and merge commit, note required-CI outcome, post one summary, and stop.

Goal: deep review plus Bugbot, fix actionable issues with the smallest correct changes, clear merge / required-CI / thread blockers, run strong local offline verification, push fixes, append the review ledger, and post one PR summary. Prefer thoroughness over speed. Regenerate large assets only when a fix requires it; then run the asset check and keep compatibility aliases byte-identical where the repo uses them.

Snapshot the GitHub head SHA: tip, base, behind/ahead, mergeable state, merge-tree versus origin/main (real conflict versus behind-but-clean), required checks on that tip including Production UI when selected, advisory separately, unresolved actionable threads. Missing checks while dirty are not green. If the tip moves mid-work, re-snapshot and continue from the new head.

Ledger-lookup against that GitHub head under the heavy review-and-fix scope for this PR. Already reviewed at this head with clean merge-tree, green required checks, and no new actionable threads → summarize, comment, stop unless I asked for a fresh superseding pass. Follow the repo review protocol.

Unblock once: real conflict → merge origin/main (prefer main’s shared queues; keep this PR’s notes); behind-but-clean → one sync, late if required CI is in flight; clean → leave. Labels like skip-branch-sync do not block a manual sync needed to clear a real blocker. No sync thrash. After any sync or push, re-snapshot tip, merge-tree, and required checks before declaring done. Dedupe the ledger if a merge touched it.

Review high-confidence delta risks only. Separate PR-introduced defects from pre-existing re-emitted debt. For generated assets, review contracts, aliases, cache, and manifests—not every generated line. If protected RAG or ranking surfaces are touched, say so before editing; fix PR-body policy text only when wrong or missing. Ignore bot noise. No nit spam or broad rewrites. Escalate verification by touched risk: clinical / RAG / privacy / migrations / auth → domain check plus production-readiness when warranted; UI / phone-chrome / routing / styling → ensure plus phone-chrome or UI gates when warranted; generated assets or docs inventory/links → asset or docs checks when warranted.

Fix P0/P1 always; clear scoped P2 when locally provable; else disposition and resolve. Required-check failures on this tip only; ignore advisory. Prefer reverting a bad autofix commit. After fixes: format and commit before push; smallest targeted proof; repo cheap gate; repo PR-local gate. No release, lighthouse, live eval, or live provider gates without approval.

Push only this PR’s fix commits. Append ledger for the final GitHub head under the heavy scope. Never push a ledger-only tip—include ledger in a real fix push, or leave a no-change ledger append local and unpushed.

One PR comment: tip, sync/merge-tree, fixed versus dispositioned, threads, required CI, decisive local gate lines, residual risks; merge left to me. Inline only for remaining human-needed P0/P1.

Stop when merge-tree is clean on the current GitHub tip, actionable threads are clear, required checks are green or clearly in progress, heavy gates for scope passed, summary posted, merge left to me.

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

@cursor

cursorBot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

PR #1530 (codex/perform-comprehensive-code-review) — tip bebc76d3. Chose this over #1531/#1532 from the GitHub PR context and current branch.

Done

  • Synced origin/main (was CONFLICTINGMERGEABLE, merge-tree clean)
  • Dropped a merge-duplicated summaryMode reject; kept main’s single early reject
  • Unique tip vs main: density native-disabled a11y + summaryMode contract test + regression pin update
  • Fixed stale aria-disabled mobile regression pins that broke verify:cheap
  • Dispositioned + resolved Qodo’s outdated pin-checker selfTest note (already on main)

Verification

  • Local: verify:cheap4700 passed; verify:pr-local build + RAG fixtures green
  • Hosted required: policy/mergeability/static/unit/build/UI critical/(2)/(3)/SAST/secrets green
  • Production UI (1) still in progress
  • Ledger append left local only (no ledger-only push)
  • Summary posted on the PR — merge left to you
Open in WebOpen in Cursor

Resolve real conflicts with origin/main. Keep main's non-stream
summaryMode rejection (already upstream) and branch-mutation pin
checks, preserve native-disabled density controls, and retain the
summaryMode contract test coverage that main still lacks.
@qodo-code-review

qodo-code-reviewBot commented Jul 31, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1)📘 Rule violations (0)📎 Requirement gaps (0)🎨 UX issues (0)🔗 Cross-repo conflicts (0)📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Unconditional pin-check self-test 🐞 Bug☼ Reliability
Description
scripts/check-github-action-pins.mjs now runs selfTest() on every normal execution, creating and
deleting temporary workflow/action files before scanning the repo. This adds unnecessary side
effects and an avoidable failure mode (tmpdir/FS restrictions) to a check that previously only read
repository files.
Code

scripts/check-github-action-pins.mjs[R97-98]

+selfTest();+failures.push(...collectPinFailures(process.cwd()));
Evidence
The script defines a selfTest() that creates a temporary .github/workflows +
.github/actions/**/action.yml tree and writes files, and then calls selfTest() unconditionally on
the normal execution path. package.json shows the check is invoked without --self-test, so the
filesystem-writing self-test always runs during npm run check:github-actions.

scripts/check-github-action-pins.mjs[64-99]
package.json[45-53]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
### Issue description
`scripts/check-github-action-pins.mjs` calls `selfTest()` unconditionally, so every run performs write/delete operations in the OS temp directory. This is unnecessary for normal scanning and can fail in more restricted environments.
### Issue Context
The script already supports `--self-test` to run the self-test explicitly and exit; unconditionally running it duplicates this capability while adding side effects.
### Fix Focus Areas
- scripts/check-github-action-pins.mjs[64-99]
### Suggested fix
- Remove the unconditional `selfTest();` call from the default path.
- Keep `--self-test` as the explicit self-test mode.
- If CI should still enforce the self-test, run `npm run check:github-actions -- --self-test` as a separate CI step (or update the npm script/workflow accordingly).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

To customize comments, go to the Qodo configuration screen, or learn more in the docs.

Qodo Logo

Comment threadscripts/check-github-action-pins.mjs
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Enforce summaryMode contract, harden GitHub Actions pinning, and a11y/docs hygiene

🐞 Bug fix✨ Enhancement🧪 Tests📝 Documentation⚙️ Configuration changes🕐 40+ Minutes

Grey Divider

AI Description

• Reject summaryMode on non-stream /api/answer and pin streamed summary scoping to the validated
documentId.
• Tighten answer request validation for summaryMode and add a contract regression test.
• Extend GitHub Actions pin checking to scan local composite actions and add a self-test; run PR
policy on release/** targets.
• Improve accessibility by using native disabled and removing href-less anchors; update DOM tests.
• Remove noisy npm config and update audit/process docs to record sweep findings and fixes.
Diagram

graph TD
Client["Client"] --> Schema["answerRequestSchema"] --> NonStream["/api/answer"] --> Rag["answerQuestionWithScope"]
Schema --> Stream["/api/answer/stream"] --> Scope["resolveSearchScope"] --> Exec["summarizeDocument / answerQuestionWithScope"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Add non-stream summary support
  • ➕ Keeps feature parity for clients that can’t/do not want to stream
  • ➕ Avoids a 400 for existing non-stream summaryMode callers
  • ➖ Duplicates summary logic and governance in a second route
  • ➖ Higher risk of future drift between stream and non-stream behaviors
  • ➖ Larger blast radius than an explicit-contract fix
2. Extract a shared answer execution module for both routes
  • ➕ Single source of truth for summaryMode routing, scoping, and execution
  • ➕ Reduces the chance of future contract drift
  • ➖ Requires a broader refactor and more test rewiring
  • ➖ Not as low-risk for a hardening/hygiene PR

Recommendation: Given the current duplication between stream and non-stream routes, explicitly rejecting summaryMode on /api/answer plus tightening schema validation is the lowest-risk way to stop silent contract drift. A follow-up refactor to share execution logic would be the most robust long-term improvement, but is appropriately deferred from this PR.

Files changed (17) +416 / -86

Bug fix (6) +55 / -23
route.tsReject summaryMode on non-stream answer endpoint+8/-0

Reject summaryMode on non-stream answer endpoint

• Adds an early 400 rejection ('summary_mode_stream_required') when 'summaryMode' is set, preventing silent fallback to normal answer generation.

src/app/api/answer/route.ts

route.tsPin summaryMode scope resolution to documentId+4/-1

Pin summaryMode scope resolution to documentId

• When 'summaryMode' is enabled, forces scope resolution to use only '[documentId]', preventing validation against one scope while summarizing a different document.

src/app/api/answer/stream/route.ts

favourites-hub.tsxUse native disabled for unavailable favourites controls+3/-3

Use native disabled for unavailable favourites controls

• Replaces 'aria-disabled="true"' with native 'disabled' on “coming soon” controls while keeping accessible descriptions, removing focusable-but-nonfunctional buttons.

src/components/clinical-dashboard/favourites-hub.tsx

differential-presentation-workflow-page.tsxAccessible explanation for disabled density controls+24/-17

Accessible explanation for disabled density controls

• Moves away from 'title="Soon"'/'aria-disabled'-only signaling by adding a visible “Density controls coming soon” message and wiring it via 'aria-describedby'.

src/components/differentials/differential-presentation-workflow-page.tsx

master-document-flow-mockups.tsxRemove href-less anchor in mockup+2/-2

Remove href-less anchor in mockup

• Replaces a styled '<a>' without 'href' (“Table 3”) with a non-interactive '<span>' to avoid misleading link semantics.

src/components/master-document-flow-mockups.tsx

answer-request.tsReject conflicting or multi-document summaryMode payloads+14/-0

Reject conflicting or multi-document summaryMode payloads

• Tightens request validation so 'summaryMode' only allows a single document and rejects mismatched 'documentId' vs 'documentIds'.

src/lib/validation/answer-request.ts

Tests (2) +60 / -12
answer-summary-mode-contract.test.tsAdd summaryMode contract regression tests+53/-0

Add summaryMode contract regression tests

• Adds tests asserting schema-level summaryMode constraints, non-stream route rejection before backend work, and stream route’s pinned document scoping.

tests/answer-summary-mode-contract.test.ts

favourites-hub-unavailable-controls.dom.test.tsxUpdate DOM tests for native disabled semantics+7/-12

Update DOM tests for native disabled semantics

• Updates assertions to expect disabled buttons (and no 'aria-disabled') while still requiring accessible descriptions for unavailable controls.

tests/favourites-hub-unavailable-controls.dom.test.tsx

Documentation (6) +223 / -35
audit-handover-2026-07-14.mdMarkdown table rendering cleanup+13/-13

Markdown table rendering cleanup

• Reformats the security/privacy/API findings table for correct markdown alignment; content remains the same.

docs/audit-handover-2026-07-14.md

repo-audit-2026-07-01.mdFix markdown escaping and table formatting+16/-16

Fix markdown escaping and table formatting

• Adjusts markdown escaping (underscores/asterisks) and table formatting so the audit renders correctly without unintended emphasis.

docs/audit/repo-audit-2026-07-01.md

repo-wide-review-sweep-2026-07-19.mdAdd repo-wide sweep findings document+188/-0

Add repo-wide sweep findings document

• Adds a new audit/sweep report capturing high-confidence findings, improvement backlog, and verification limitations from a broad static review pass.

docs/audit/repo-wide-review-sweep-2026-07-19.md

database-drift-detection.mdEscape identifiers to prevent markdown italics+2/-2

Escape identifiers to prevent markdown italics

• Fixes accidental markdown emphasis in technical identifiers (underscores/asterisks) to keep documentation accurate.

docs/database-drift-detection.md

ingestion-concurrency-fix-workorder.mdEscape technical identifiers in prose+3/-3

Escape technical identifiers in prose

• Fixes markdown escaping so identifiers like 'section_index' render literally; also cleans up emphasis markers.

docs/ingestion-concurrency-fix-workorder.md

process-hardening.mdEscape env var name for correct rendering+1/-1

Escape env var name for correct rendering

• Escapes underscores/asterisks in an E2E env var reference to avoid unintended markdown formatting.

docs/process-hardening.md

Other (3) +78 / -16
pr-policy.ymlRun PR policy workflow for release branches+1/-1

Run PR policy workflow for release branches

• Extends the 'pull_request_target' trigger to include 'release/**' so release-targeted PRs receive the same metadata/evidence governance checks as 'main' PRs.

.github/workflows/pr-policy.yml

.npmrcRemove unknown npm config entry+0/-1

Remove unknown npm config entry

• Deletes 'allow-scripts=true', which npm reports as an unknown project config and can create noisy/fragile installs.

.npmrc

check-github-action-pins.mjsScan composite actions and add a self-test harness+77/-14

Scan composite actions and add a self-test harness

• Extends pin checking to include '.github/actions/**/action.yml' files in addition to workflow YAMLs, refactors discovery into reusable functions, and adds a temporary-fixture self-test (also runnable via '--self-test').

scripts/check-github-action-pins.mjs

Update mobile interaction regressions to expect native disabled
density placeholders without redundant aria-disabled attributes.
@ecc-tools

ecc-toolsBot commented Jul 31, 2026

Copy link
Copy Markdown

🔒 Upgrade Required

Private repository analysis requires Pro or Enterprise.

Upgrade: https://ecc.tools/pricing?plan=pro


ECC Tools keeps the core app open, and puts private repos, team features, and enterprise controls behind paid tiers.

@BigSimmo
BigSimmo enabled auto-merge (squash) July 31, 2026 11:38
@cursor

cursorBot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

PR #1530 end-to-end summary

Tip:bebc76d3a332493e4fcaf076371ba7150123bf20 on codex/perform-comprehensive-code-review
Base:origin/main @ ed849c7f
Merge-tree: clean (MERGEABLE; was CONFLICTING/DIRTY at start). Ahead 3 / behind 0.

Sync

  • Merged origin/main once to clear real conflicts in scripts/check-github-action-pins.mjs, answer-request.ts, favourites DOM tests, and differentials density controls.
  • After sync, most of the original Codex delta was already on main; unique tip vs main is:
    • src/components/differentials/differential-presentation-workflow-page.tsx (native disabled, no redundant aria-disabled)
    • tests/answer-summary-mode-contract.test.ts
    • tests/mobile-interaction-regressions.test.ts (pins updated to match)

Fixed

  • Merge conflicts resolved (prefer main’s shared queues / consolidated summaryMode validation / pin branch-mutation guards).
  • Removed merge-duplicated non-stream summaryMode rejection (kept main’s single early reject).
  • Updated mobile density-control source pins after verify:cheap caught the stale aria-disabled expectation.

Dispositioned

  • Qodo “unconditional selfTest()” on check-github-action-pins.mjs: pre-existing on main, not unique to this tip (diff empty); outdated thread replied + resolved.
  • No Bugbot findings (Codex review usage capped; CodeRabbit rate-limited; Qodo only that outdated note).

Threads

  • Actionable unresolved threads: 0

Required CI (tip bebc76d3)

  • Green: PR policy, PR mergeability, Change scope, Static PR checks, Unit coverage, Build, Safety/config, Production UI critical, Production UI (2)/(3), Semgrep, Semgrep ingestion gate, Gitleaks, GitGuardian
  • In progress: Production UI (1) (Chromium journeys running)
  • Advisory ignored: Lighthouse / Visual baselines as applicable

Local heavy gates

  • npm run verify:cheap — 450 files / 4700 passed / 4 skipped
  • npm run verify:pr-local — build + client secret scan + offline RAG 36 fixtures / 574 tests passed
  • Focused: answer summaryMode contract + favourites + mobile regressions 8/8
  • No provider-backed / release / live-eval gates

Residual risks

  • Hosted Production UI shard (1) still finishing; watch that job before merge.
  • Review ledger row for this head appended locally only (no ledger-only push).
  • Merge left to you.

@BigSimmo
BigSimmo merged commit 6f94ae8 into mainJul 31, 2026
33 checks passed
@BigSimmo
BigSimmo deleted the codex/perform-comprehensive-code-review branch July 31, 2026 11:52
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@BigSimmo@cursoragent