feat(forms): answer-first grouped result cards for phone search results - #1514
Conversation
The phone results list nested a bordered card inside a bordered results
panel, repeated the statutory category as a chip on every card, and gave
each card an Open button duplicating the row's own navigation. Three
container levels and two competing badges left roughly three results above
the fold, and a query that names one specific form ("1A") looked identical
to a broad text search.
Restructure the mobile list only:
- Lead with an exact-form-code hero when the query resolves to a form code
("1A", "form 1a", "1A attachment"). It carries the form's purpose, risk,
category, availability, and a wired Open action; the remaining results
drop to "Also references <code>" below.
- Drop the enclosing searchResultsSection so cards sit directly on the page
canvas. Nothing is nested inside another bordered surface.
- Make the whole card the Link and remove the per-card Open button.
- Group results under their statutory category, stated once in a group
header instead of repeated as a chip on each card. Groups are emitted in
order of first appearance so the active sort still decides which group
leads.
- Leave risk as the only row badge, driven by the catalogue's typed
riskLevel with a status-chip fallback so the safety signal cannot silently
disappear for records without a catalogue payload.
- Replace the "Content match in record details" line, which read the same on
every card, with the form's purpose clamped to two lines — but only where
the catalogue holds real editorial prose. Most forms fall back to
generated boilerplate that restates the title ("Official form source:
Transfer Order. Review the source snippets…"); showing that would swap one
piece of non-information for a longer one, so those cards stay title-only.
Narrow one guard in ui-tools.spec.ts. It asserted the mobile results never
contain /pathway/i as a proxy for the unsubstantiated pathway-claims feature
(supportsPathwayClaims). Cards now carry catalogue purpose text, and several
official purposes legitimately say "pathway" ("Use for each leave episode
from inpatient treatment order pathway"), so the proxy fired on real
content. It now matches the pathway panel's own strings; the page-level
exact guard on the next line is unchanged.
The app shell header, phone composer, SearchResultsHeaderBand, and the
desktop ResultsTable are unchanged. Presentation only: no retrieval,
ranking, or ordering behaviour is touched.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Hb5uoe2z1E6oKLdPKNyzFRWarning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in:16 minutes Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughMobile forms search now supports exact form-code matches, grouped related results, risk and purpose metadata, and updated mobile result assertions. ChangesMobile forms search
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant MobileSearch
participant CatalogueMetadata
participant ResultGroups
participant FormPage
MobileSearch->>CatalogueMetadata: retrieve form metadata
CatalogueMetadata-->>MobileSearch: return code, purpose, risk, and category
MobileSearch->>ResultGroups: separate exact match and group related results
ResultGroups-->>MobileSearch: return hero and category cards
MobileSearch->>FormPage: open selected form
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
BigSimmo
commented
Jul 31, 2026
Temporarily closing to preserve the serialized merge queue while #1510 exact-head CI runs. This PR will be reviewed in the next queue slot. |
BigSimmo
commented
Jul 31, 2026
@coderabbitai please run a new review of this closed PR for me |
✅ Action performedReview finished.
|
BigSimmo
commented
Jul 31, 2026
@cursoragent Prep this closed PR for reopen. Leave it CLOSED.
Treat closed-PR CI as stale. Don’t run local suites CI will cover unless |
PR #1514 is ready to reopen and left CLOSED. Snapshot (before → after)
High-risk surfacesNone. Diff is phone forms presentation + one Playwright guard: SyncFetched Tip review (vs base)No P0–P2 findings. Exact-code hero ( Residual risks
Ledger / hygiene
  |
…-mockups-iz7owo Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Record sync + tip review for reopen readiness. Leave PR closed. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
This pull request has been ignored for the connected project Preview Branches by Supabase. |
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
BigSimmo
commented
Jul 31, 2026
@cursoragent This PR was just reopened. Do not merge, force-push, rebase, or arm auto-merge
|
Reopen follow-up complete — merge decision left to you. 1. Tip confirmationGitHub head matches the branch tip:
2. SyncWas behind 5 + GitHub 3. CI on this tipIgnoring pre-close rollup. On
4. ThreadsNone from Codex/Bugbot/Copilot/human. Codex usage-limit / CodeRabbit draft-skip noise ignored. Residual risks
No merge, force-push, rebase, or auto-merge changes from me. |
…-mockups-iz7owo Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/components/forms/forms-search-results-page.tsx (1)
641-653: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider extracting the risk badge into a shared helper.
The risk badge markup at Line 641-653 duplicates the one in
MobileExactMatchHero(Line 573-578), differing only in wrapper size. Extracting a smallRiskBadge({ risk })component would remove the duplication.🤖 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/components/forms/forms-search-results-page.tsx` around lines 641 - 653, Extract the duplicated risk badge markup from the current results rendering and MobileExactMatchHero into a shared RiskBadge({ risk }) component, preserving the existing riskBadgeToneClass styling, displayed risk text, and sr-only “risk” label. Allow the two call sites to retain their distinct wrapper sizing through an appropriate prop or surrounding class while keeping badge behavior unchanged.
🤖 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 `@docs/branch-review-ledger.md`:
- Line 349: Remove the manually added row from docs/branch-review-ledger.md.
Record the entry using the repository’s approved ledger lookup/write tooling
instead, without directly reading or editing the ledger file.
In `@src/components/forms/forms-search-results-page.tsx`:
- Around line 668-676: Update the results section around the exactMatch heading
and remaining groups so the heading, count, and groups block render only when
remaining.length is greater than zero. Keep MobileExactMatchHero rendering
independently so it can stand alone when no remaining results exist.
- Line 549: Update the availabilityLabel logic in the forms search results page
to stop reading statusChips[2]. Derive availability from the reliable
details.availability or an explicit availability field, and omit the label when
no reliable value is present.
---
Nitpick comments:
In `@src/components/forms/forms-search-results-page.tsx`:
- Around line 641-653: Extract the duplicated risk badge markup from the current
results rendering and MobileExactMatchHero into a shared RiskBadge({ risk })
component, preserving the existing riskBadgeToneClass styling, displayed risk
text, and sr-only “risk” label. Allow the two call sites to retain their
distinct wrapper sizing through an appropriate prop or surrounding class while
keeping badge behavior unchanged.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: bcd01105-6f5d-4b8d-8a75-a8c7470131f9
📒 Files selected for processing (3)
docs/branch-review-ledger.mdsrc/components/forms/forms-search-results-page.tsxtests/ui-tools.spec.ts
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 1 file(s) based on 3 unresolved review comments. Files modified:
Commit: The changes have been pushed to the Time taken: |
CI triageCI failed on this PR. Automated classification of the 2 failed job(s):
Compared with main CI run #7441 (cancelled). Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger. |
Fixed 1 file(s) based on 3 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
Uh oh!
There was an error while loading. Please reload this page.
Summary
MobileCardsinsrc/components/forms/forms-search-results-page.tsx). The list previously nested a bordered card inside a bordered results panel, repeated the statutory category as a chip on every card, and gave each card anOpenbutton that duplicated the row's own navigation — three container levels and two competing badges, leaving roughly three results above the fold.1A,form 1a,1A attachment). It carries the form's purpose, risk, category, availability, and a wiredOpen Form <code>link; the remaining results move underAlso references <code>. A query that names one specific form no longer looks identical to a broad text search.searchResultsSectionso cards sit directly on the page canvas — no bordered card nested inside another bordered surface. This matches the card-on-canvas pattern already used byfavourites-hubandservices-navigator-page.Linkand remove the per-cardOpenbutton; in a ranked list the card is the action.FormCatalogDetails.category), stated once in a group header rather than repeated as a chip on every card. Groups are emitted in order of first appearance, so the existing sort (relevance / A–Z) still decides which group leads and the top-ranked form stays in the top group.riskLevel, with a status-chip fallback so the safety signal cannot silently disappear for records that reach the registry without a catalogue payload.Content match in record detailsline — which read identically on every card — with the form'spurpose, clamped to two lines. Only genuine editorial prose is shown: most forms fall back to generated boilerplate that restates the title (Official form source: Transfer Order. Review the source snippets…), and rendering that would swap one piece of non-information for a longer one, so those cards stay title-only.tests/ui-tools.spec.ts(detail under Risk and rollout below).Not changed, deliberately: the app shell header, the phone composer/footer,
SearchResultsHeaderBand(query echo, match count, Relevance/A–Z, Results/Forms tabs), and the desktopResultsTable.Verification
npm run verify:cheap—Test Files 434 passed (434),Tests 4563 passed | 4 skipped (4567).npm run verify:ui—340 passed,2 failed. Both failures are pre-existing and environmental, measured against the parent commitfcd9041in a clean worktree where they fail identically:ui-pwa.spec.ts:143(Chromium reports"in-incognito"installability) andui-smoke.spec.ts:3614(document viewer PDF preview). See the note on the browser binary below.4 passed:forms search mockup is usable without horizontal overflow on mobile,phone bottom search dock stays edge-to-edge with safe-area padding inside the form,tablet and desktop forms results keep non-phone bottom clearance,forms mode home never overflows horizontally across sizes.scrollWidth === clientWidth === 393.npm run eval:retrieval:quality,eval:rag, andcheck:production-readinesswere not run: this change touches presentation only and no retrieval, ranking, selection, scoring, or answer-generation code.classifyPullRequestFilesreturnsclinicalRisk: falsefor this path and it is not a RAG-ranking surface.Browser-binary note:
verify:uicannot run unmodified in this container — Playwright 1.62.0 expects browser revision1234and the image ships1194, so every test fails atbrowserType.launch. All runs above usedPLAYWRIGHT_CHROMIUM_EXECUTABLE_PATH=/opt/pw-browsers/chromium, the overrideplaywright.config.tsalready provides for sandboxed containers. The two residual failures are attributable to that substituted binary and reproduce on the parent commit.Risk and rollout
findExactFormCodeMatchonly fires on an exact normalised match againstFormCatalogDetails.form, and when it fires the hero form is removed from the list below so it is never shown twice. If no code matches, the page renders exactly as the grouped list with aBest matchesheading.tests/ui-tools.spec.ts:1202asserted the mobile results never contain/pathway/i, as a broad proxy for the unsubstantiated pathway-claims feature (supportsPathwayClaims, currentlyfalse). Result cards now carry cataloguepurposetext, and several official purposes legitimately contain the word — e.g.Use for each leave episode from inpatient treatment order pathway— so the proxy began firing on real clinical content rather than on the feature. It now matches the pathway panel's own strings (related pathway,view full pathway). The page-level exact guard on the following line (PSOLIS Transport|View full pathway|Source verified) is unchanged, and that is the assertion that actually pins the feature's absence. Flagging explicitly because weakening a guard is normally the wrong move; happy to revert this hunk and drop the purpose line instead if preferred.Notes
ResultsTablestill shows the per-rowOpenbutton and category tags, and has no exact-code hero. That was outside the scope of this change and is the obvious follow-up.line-clamp-2 blockdoes not clamp. Theblockutility overrides thedisplay: -webkit-boxthat line-clamp requires. Hit while building this; the same pairing appears atformulation-map-page.tsx:121andformulation-builder-page.tsx:139,423, which are likely not clamping either. Not touched here.category,purpose,riskLevel, availability status chip) — no clinical text was authored for this change.eslint-rules/require-button-wiring.mjsand the documented "Language and region" defect.🤖 Generated with Claude Code
https://claude.ai/code/session_01Hb5uoe2z1E6oKLdPKNyzFR
Generated by Claude Code
Summary by CodeRabbit