Skip to content

fix(documents): recount facets against the current selection, and capture five search-filter findings - #1526

Merged
BigSimmo merged 8 commits into
mainfrom
claude/top-search-design-mockups-w53znc
Jul 31, 2026
Merged

fix(documents): recount facets against the current selection, and capture five search-filter findings#1526
BigSimmo merged 8 commits into
mainfrom
claude/top-search-design-mockups-w53znc

Conversation

@BigSimmo

@BigSimmoBigSimmo commented Jul 31, 2026

Copy link
Copy Markdown
Owner

Summary

Two product commits plus a main merge that renumbered the ledger, and a small follow-up that disables dead-end facets.

cf5d7cdf — fix the stale facet counts (archived as #173).buildSmartDocumentTagFacetIndex counted every facet once against the whole match set and never revised it. Selections AND together (filterDocumentsBySmartTagFacetIndex), so the moment one facet was applied every other one still reported a number for a set the reader was no longer looking at — and some pointed at combinations returning nothing.

projectSmartTagFacetGroups(index, selectedTagKeys) re-counts an already-built index against the live selection. Each count answers: how many documents would I have if I ticked this as well? An already-selected facet reports the current result count.

  • Membership and order are preserved (no jump under the pointer).
  • A facet falling to zero stays visible at zero so the UI can disable it.
  • No-op when nothing is selected (same array identity).

a919bf98 — capture five search-filter findings in docs/outstanding-issues.md. After merging main (which claimed #169 for unpushed local branches), these are #170#174: phone filter sheet, four overlapping filtering surfaces, Sources as navigation, the stale-count defect (archived #173), and AND-within-group as a product decision.

9f6cf337 — merge main, resolving the outstanding-issues id collision without dropping either side's rows.

51eae876 — disable zero-count unselected facet buttons in the documents tag rail so dead-end combinations cannot be selected.

Verification

  • npm run verify:cheap — exit 0 (Test Files 449 passed, Tests 4697 passed | 4 skipped)
  • npm run typecheck clean; npm run format run and committed
  • tests/document-tags.test.ts — 16 passed (six new projection cases; mutation-verified)
  • npm run check:outstanding-issues172 rows (56 open, 116 archived), next-id=175
  • git merge-tree --write-tree origin/main HEAD clean; GitHub mergeable: MERGEABLE
  • UI verification not run locally (Chromium pin mismatch); Production UI gates in CI

Risk and rollout

  • Risk: low, confined to documents mode. Only facet counts (and disable of zero-count unselected rows) change while a facet is selected. Filtering itself is unchanged — filterDocumentsBySmartTagFacetIndex is unmodified.
  • Rollback: revert the facet-count and disable commits (or their squash hunks after merge).
  • Provider or production effects: None.

RAG impact: no retrieval behaviour change — display/disable of facet counts only; no src/lib/rag/**, clinical-search, retrieval-selection, ranking, eval, or golden fixture changes.

Clinical Governance Preflight

  • Source-backed claims still require linked source verification before clinical use
  • No patient-identifiable document workflow was introduced or expanded without explicit governance approval
  • Supabase target remains Clinical KB Database (sjrfecxgysukkwxsowpy)
  • Service-role keys and private document access remain server-only
  • Demo/synthetic content remains clearly separated from real clinical sources
  • Source metadata, review status, and outdated/unknown-source behavior remain conservative
  • Deployment classification/TGA SaMD impact was checked when clinical decision-support behavior changed

Presents counts over an already-retrieved result set. No retrieval, ranking, or source-selection change. More conservative: dead-end facets report zero and cannot be selected.

Notes

Batch A from PR #1523. Still outstanding after renumber:

  • #171 + #172 — merge the four filtering surfaces; move Sources to nav as Browse library
  • #170 — adopt ui/sheet.tsx for phone filter controls
  • #174 — OR-within-group product decision

This file is a CI sync template only. A follow-up commit on this branch deletes it so squash-merge does not leave a leftover body template on main.

…nd work
PR #1523 shipped the band changes; these are what it deliberately left out,
plus two defects found while tracing the filter pipeline. All five existed only
in a chat session until now.
- #169 phone filter controls render inline in a scrolling rail; the repo
already has the right primitive in ui/sheet.tsx, so this is adoption
- #170 documents mode has four overlapping filtering surfaces, and two of them
are the same job — the source-type control duplicates the Document type
facet group, and "Tables" appears in both it and the scope chips
- #171 Sources sits in the results bar but is navigation, not a filter; it is
also documents-mode only, so it can never be a fixture of a shared band
- #172 facet counts come from `matches` while format counts come from
`visibleMatches`, so half the filter panel goes stale on first selection and
points at combinations that return nothing
- #173 facets AND within a group, so two values from one group almost always
return zero; flagged as a product decision rather than changed
Numbered #169-#173 after main took #165-#168 while these were being written —
the read-modify-write allocation race #156 already records.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Akwz3Sdms8uJ5AkDt3CduY
@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 ↗︎.

@coderabbitai

coderabbitaiBot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

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:34 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: 352d3fad-3e96-4ef1-9a71-cb81f350f79b

📥 Commits

Reviewing files that changed from the base of the PR and between 9f6cf33 and 8a406c8.

📒 Files selected for processing (3)
  • PR_POLICY_BODY.md
  • docs/outstanding-issues.md
  • src/components/clinical-dashboard/document-search-results.tsx
📝 Walkthrough

Walkthrough

The PR adds selection-aware smart-tag facet projection, uses it in document search results, adds fixture-driven tests, and updates the outstanding-issues ledger.

Changes

Selection-aware facet projection

Layer / File(s)Summary
Facet projection logic and validation
src/lib/document-tags.ts, tests/document-tags.test.ts
Adds projectSmartTagFacetGroups and tests recalculated counts, selected and incompatible facets, empty selections, filtering consistency, and stable ordering.
Document search integration
src/components/clinical-dashboard/document-search-results.tsx
Uses the projection helper to derive displayed tag facet groups from active facet keys.
Issue ledger updates
docs/outstanding-issues.md
Advances the issue marker, adds open items, and records the resolved facet-count fix.

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

Possibly related PRs

Suggested reviewers:claude

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check nameStatusExplanation
Docstring Coverage✅ PassedNo functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.
Title check✅ PassedThe title clearly summarizes the two main changes: facet recounting and documentation of five search-filter findings.
Description check✅ PassedThe description includes the required summary, verification, risk and rollout, governance, and notes sections with specific evidence.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

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

@BigSimmo
BigSimmo marked this pull request as ready for review July 31, 2026 09:59
@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.

Facet counts were built once against the whole match set and never revised.
Selections AND together, so the moment one was applied every other facet was
still reporting a number for a set the reader was no longer looking at — and
some of those numbers pointed at combinations that return nothing, which reads
as a live option and behaves as a dead end.
`projectSmartTagFacetGroups` re-counts an already-built index against the live
selection. Each count now answers the question the row actually poses: how many
documents you would have if you ticked this as well. A facet that is already
selected reports the current result count, because that is what it is giving
you.
Membership and order are deliberately preserved rather than re-sorted by the
new counts: re-sorting would make rows jump under the pointer while selecting,
and re-slicing to the top N could drop a facet already ticked. A facet whose
count falls to zero stays in place at zero so the caller can disable it —
removing it makes the list jump and hides the reason it went away.
The projection is a no-op returning the same array identity when nothing is
selected, so the unfiltered path is unchanged.
Tests: six cases covering the stale count, the selected-facet count, the
dead-end combination driven to zero, agreement between each count and the
filter it describes, and stability of membership and order. Verified by
mutation — neutering the projection fails three of them.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Akwz3Sdms8uJ5AkDt3CduY
@BigSimmo

Copy link
Copy Markdown
OwnerAuthor

@cursoragent resolve merge conflict and fix any issues and run bugbot

@cursor

cursorBot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Merge conflict on PR #1526 is resolved and the branch is MERGEABLE again.

Conflict: only docs/outstanding-issues.md (no code conflicts). Kept main’s #169 (unpushed local branches), renumbered this PR’s captures to #170#174, archived the fixed facet-count bug as #173, set issues:next-id=175. check:outstanding-issues passed.

Follow-up fix: zero-count unselected tag facets are now disabled in the documents rail (51eae876), matching the projection helper’s intent.

Bugbot: no cursor[bot] findings on the tip. Local hunt found no P0/P1; the disable fix closed the remaining dead-end-click gap. Cursor’s Bugbot suite auto-queues on push (no force-trigger in this repo).

Tip:51eae876 · CI re-running · #1526

Open in WebOpen in Cursor

@BigSimmoBigSimmo changed the title docs(issues): capture five search-filter findings from the results-band workfix(documents): recount facets against the current selection, and capture five search-filter findingsJul 31, 2026
@BigSimmoClaude

Copy link
Copy Markdown
OwnerAuthor

Note for whoever resolves this — the conflict is a single file and it is one with a documented resolution protocol that a generic merge will get wrong.

git merge-tree --write-tree origin/main HEAD
CONFLICT (content): Merge conflict in docs/outstanding-issues.md

Branch is behind by 1. No code file conflicts — src/lib/document-tags.ts, src/components/clinical-dashboard/document-search-results.tsx and tests/document-tags.test.ts all merge clean.

Why this file is special. It deliberately has no merge driver#133 tried merge=union and removed it, precisely so overlapping edits conflict loudly instead of being silently concatenated. So the conflict is the guard working, not a defect.

How it must be resolved (this is #156, and it has already caused row loss once):

  1. Do not take either side wholesale. Both sides' rows are real and must survive. Taking "ours" drops main's new rows; taking "theirs" drops #169#173 from this branch.
  2. Re-derive the ids from main. The cause is read-modify-write allocation against the <!-- issues:next-id=N --> marker: main has taken ids since this branch wrote its rows, so this branch's #169#173 now collide and need renumbering above main's highest, with their cross-references updated to match. That renumbering already happened once on this branch (#165#168#169#173).
  3. The marker must end up above the highest id present, and there are legitimately two next-id comments in this file — the second is not corruption.
  4. npm run check:outstanding-issues is the only thing that catches a bad resolution.git merge reports success on a file where rows have been dropped or ids duplicated; the guard does not. Run it before pushing.

Prior evidence that this is not theoretical: a GitHub Update branch auto-merge on PR #1451 produced two rows numbered #141 and two next-id markers, leaving the marker belowmain's highest id so the next allocation would have reused a live number. git merge said success. Separately, a second session resolving this file dropped an entire appended evidence block, which the guard cannot detect at all — it validates ids and structure, not whether a merge kept both sides' prose.

I have not pushed a resolution, to avoid two agents writing the same branch (#155). Happy to take it if that is preferred — say so and I will.


Generated by Claude Code

cursoragentand others added 2 commits July 31, 2026 10:01
Keep main's #169 (unpushed local branches). Renumber this branch's
search-filter captures to #170-#174, archive the fixed facet-count
defect as #173, and advance issues:next-id to 175.
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
projectSmartTagFacetGroups leaves dead-end facets visible at count 0
so the rail does not jump; the rail now disables those buttons so they
cannot be selected. Selected facets stay clickable so filters can be
cleared.
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>

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

🤖 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 `@src/components/clinical-dashboard/document-search-results.tsx`:
- Around line 925-932: Update the facet-button configuration passed to
DocumentTagFacetRail so unselected facets with a projected count of zero are
disabled, using tagFacetGroups from projectSmartTagFacetGroups. Keep selected
facets enabled regardless of count so users can remove them, and preserve
existing behavior for nonzero or selected facets.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 7317fd19-86d8-4db5-82e8-e5c093b529de

📥 Commits

Reviewing files that changed from the base of the PR and between 2b04df4 and 9f6cf33.

📒 Files selected for processing (4)
  • docs/outstanding-issues.md
  • src/components/clinical-dashboard/document-search-results.tsx
  • src/lib/document-tags.ts
  • tests/document-tags.test.ts

@BigSimmo

Copy link
Copy Markdown
OwnerAuthor

@cursoragent Work this open PR (confirm number + GitHub head first).

Start from origin/<branch> matching that head; preserve unrelated local WIP. Do not merge, force-push, rebase, or arm auto-merge. No provider-backed gates without approval. If you can’t push or resolve threads: diagnose + comment only. If auto-merge is already armed, push only for a real blocker.

Goal: review + Bugbot the tip, fix actionable issues with smallest changes, clear merge/required-CI/thread blockers, push fixes, post one PR summary. Stay light — no full local verify:* / Playwright / broad suites unless needed to prove one concrete fix. Don’t regenerate large assets unless required.

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

  1. Snapshot tip, behind/ahead, mergeable, merge-tree (real conflict vs behind-but-clean), required checks on that tip, unresolved actionable threads. Missing checks while dirty ≠ green. If the tip moves mid-work, re-snapshot.
    npm run ledger:lookup -- <branch> --scope "PR #<n> review+bugbot+fix".
    If already reviewed at this HEAD/scope and still clean with required checks green and no new actionable threads: summarize, comment, stop.

  2. Unblock once: real conflict → merge origin/main (for ledger/issues churn, prefer main’s queues, keep this PR’s notes); behind-but-clean → sync once (wait if required CI is in flight); else leave. No sync thrash.

  3. Review + Bugbot the delta vs base. High-confidence only (correctness, security, privacy, auth, data loss, migrations/RLS, clinical/RAG/source, prod reliability, user-facing). Separate PR-introduced defects from pre-existing re-emitted data. For generated assets, review generator/contracts/aliases/cache — not every generated line. If RAG/governance surfaces are touched, note/fix PR body only when policy text is wrong or missing. Ignore bot summary/rate-limit noise.

  4. Fix P0/P1 always; fix clear scoped P2; otherwise short disposition reply then resolve. Fix required-check failures on this tip only (ignore advisory). Prove with the smallest targeted check only. No broad refactors.

  5. Format + commit before push. Push only this PR’s fix commits. Append ledger for the final GitHub head + scope above (--supersede on a requested fresh same-HEAD pass). Never push a ledger-only tip.

  6. Post one concise top-level PR comment: tip SHA, sync, fixed vs dispositioned, required CI status, residual risks. Leave merge to me. Inline comments only for remaining open P0/P1 needing a human decision.

Stop when merge-tree clean, no actionable threads left, and required checks are green or clearly in progress on the current tip.

@cursor

cursorBot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

PR #1526 · tip 741dad3d · MERGEABLE · merge-tree clean · 0 behind

Done

  • Review + Bugbot: no cursor[bot] findings; no P0/P1 in the facet-recount delta. CodeRabbit zero-count disable already fixed (51eae876); thread resolved.
  • PR body: leftover PR_POLICY_BODY.md (from Add privacy-safe optional Sentry error-tracking and surface source-governance metadata in RAG prompts #1478 on main) was overwriting the description with Sentry/RAG text. Synced the correct facet body, then deleted the file (96a66a72741dad3d) so Static Prettier fails and main pollution both stop.
  • No sync thrash: already even with main; auto-merge not armed.
  • Summary comment posted on the PR.

CI

Required checks re-running on 741dad3d after the template cleanup (in progress). Prior tip had Static/Build/Unit/Production UI critical green before those body fixes.

Residual

  • Merge left to you.
  • Ledger row not pushed (would be a ledger-only tip).
Open in WebOpen in Cursor

@BigSimmo
BigSimmo enabled auto-merge (squash) July 31, 2026 10:12
cursoragentand others added 2 commits July 31, 2026 10:13
Sync PR policy body was overwriting this PR's description with leftover
Sentry/RAG template text from main (#1478). Point the template at the
actual facet-recount work so CI can sync an accurate body.
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Sync already applied the correct #1526 description. Removing the
template clears the Static PR Prettier failure and stops squash-merge
from leaving a stale body file on main (#1478 leftover).
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
@cursor

cursorBot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

PR #1526 review+bugbot+fix

Tip:741dad3d · Branch:claude/top-search-design-mockups-w53znc · Base:main
Sync: merge-tree clean vs origin/main, 0 behind / 6 ahead, MERGEABLE (auto-merge not armed)

Fixed

Dispositioned / no action

  • Bugbot: 0 cursor[bot] findings on tip.
  • Delta review: no P0/P1. Facet projection + tests look sound; filtering path unchanged.
  • Pre-existing: facet rail only mounts when activeFacetKeys.length > 0 (seeded from card tags) — out of this PR’s delta / tracked under filter-surface follow-ups.

Required CI (on 741dad3d)

Re-running after the body/template cleanup. Prior tip 51eae876 had Static/Build/Unit/Production UI critical green before the body-fix pushes. Static PR had failed once on Prettier of PR_POLICY_BODY.md — cleared by deleting that file.

Residual

  • Leave merge to you.
  • main still carried PR_POLICY_BODY.md until this tip deletes it; other open PRs may still show the old Sentry body until they sync past this merge or drop their own copy.

@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 merged commit ed849c7 into mainJul 31, 2026
25 checks passed
@BigSimmo
BigSimmo deleted the claude/top-search-design-mockups-w53znc branch July 31, 2026 11:00
Sign up for freeto 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.

3 participants

@BigSimmo@claude@cursoragent