Skip to content

Fix service-related options placement and test sync (clean tree)#844

Closed
BigSimmo wants to merge 2 commits into
mainfrom
codex/design-audit-main-safe-20260718-separate-clean2
Closed

Fix service-related options placement and test sync (clean tree)#844
BigSimmo wants to merge 2 commits into
mainfrom
codex/design-audit-main-safe-20260718-separate-clean2

Conversation

@BigSimmo

@BigSimmo BigSimmo commented Jul 18, 2026

Copy link
Copy Markdown
Owner

Summary

  • Move service-related option controls out of the SearchResultsLayout header on services pages so related options render directly beneath service results as expected.
  • Align current affected UI tests to match runtime behavior, including Clinical Guide title variant and mobile/service selector stability.

Verification

  • npx playwright test tests/ui-accessibility.spec.ts --project=chromium --workers=1
  • npx playwright test tests/ui-universal-search.spec.ts --project=chromium --workers=1
  • npm run test -- tests/site-map.test.ts tests/supabase-schema.test.ts

Risk and rollout

  • Scope is limited to navigator layout and UI test assertions.
  • Regression risk is low and localized to these surfaces.

Clinical Governance Preflight

  • No clinical logic edits in this change.
  • No source, ingestion, or answer-generation behavior edits.
  • No production config/env/provider credential changes.
  • Rollback: Revert this PR.

Summary by CodeRabbit

  • Bug Fixes

    • Improved comparison panel updates when the number of selected items changes.
    • Limited “also matches” suggestions to applicable search result views, avoiding display during loading, blocked, or empty states.
  • Tests

    • Updated navigation, search, UI, schema, and route coverage to reflect current behavior and validation rules.

@supabase

supabase Bot commented Jul 18, 2026

Copy link
Copy Markdown

This pull request has been ignored for the connected project sjrfecxgysukkwxsowpy because there are no changes detected in supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.


Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

@coderabbitai

coderabbitai Bot commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The PR adjusts service navigation rendering, updates navigation and Supabase SQL contract assertions, aligns a sitemap route placeholder, and refreshes search UI regression test expectations.

Changes

Navigation and regression contracts

Layer / File(s) Summary
Service navigation rendering
src/components/services/services-navigator-page.tsx
RightRail remounts across selection-count states, while UniversalSearchAlsoMatches renders only in the results branch.
Navigation and auth contracts
tests/audit-navigation-auth-regressions.test.ts, tests/site-map.test.ts
Assertions now cover local-project readiness, the updated polling refresh condition, and the [workflow-slug] presentations route placeholder.
Supabase schema contracts
tests/supabase-schema.test.ts
Migration and SQL expectations are updated for ACL privileges, registry cleanup, query-term correction, and trigram index creation.
Search UI regression contracts
tests/ui-tools.spec.ts, tests/ui-universal-search.spec.ts
Search test wording, phone dock transition coverage, and the universal-search query fixture are updated.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

  • BigSimmo/Database#749: Changes the rendering conditions for UniversalSearchAlsoMatches in the same service navigation flow.
  • BigSimmo/Database#735: Updates the same RightRail selection-count remount behavior and related navigation regression expectations.
  • BigSimmo/Database#815: Overlaps on RightRail behavior and Supabase schema assertions for query correction and trigram indexing.

Suggested labels: codex

Suggested reviewers: copilot, claude

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly reflects the main change: service option placement and related test sync.
Description check ✅ Passed The description covers summary, verification, risk, and governance sections and is sufficiently complete.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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 codex/design-audit-main-safe-20260718-separate-clean2

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

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

🧹 Nitpick comments (1)
tests/supabase-schema.test.ts (1)

1399-1401: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Tautological assertion — pg_trgm.similarity_threshold check can never fail.

The pattern if (corrector.includes(X)) { expect(corrector).toContain(X); } is a tautology: if includes returns true, toContain trivially passes; if false, the assertion is skipped entirely. This provides zero verification value. If the intent is to assert the similarity threshold only when present, consider making it an unconditional assertion or at minimum logging when the guard skips.

♻️ Suggested refactor: make the assertion unconditional
-      if (corrector.includes("set pg_trgm.similarity_threshold = 0.3")) {
-        expect(corrector).toContain("set pg_trgm.similarity_threshold = 0.3");
-      }
+      expect(corrector).toContain("set pg_trgm.similarity_threshold = 0.3");
🤖 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/supabase-schema.test.ts` around lines 1399 - 1401, Replace the
tautological conditional around the pg_trgm.similarity_threshold check with an
unconditional assertion in the relevant test, so the expected setting is always
verified. Remove the redundant corrector.includes guard while preserving the
existing expected threshold string.
🤖 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.

Nitpick comments:
In `@tests/supabase-schema.test.ts`:
- Around line 1399-1401: Replace the tautological conditional around the
pg_trgm.similarity_threshold check with an unconditional assertion in the
relevant test, so the expected setting is always verified. Remove the redundant
corrector.includes guard while preserving the existing expected threshold
string.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: bf5c92a3-1963-41b1-89a0-465cdcc6d829

📥 Commits

Reviewing files that changed from the base of the PR and between 7815204 and c08a03f.

📒 Files selected for processing (6)
  • src/components/services/services-navigator-page.tsx
  • tests/audit-navigation-auth-regressions.test.ts
  • tests/site-map.test.ts
  • tests/supabase-schema.test.ts
  • tests/ui-tools.spec.ts
  • tests/ui-universal-search.spec.ts

@github-actions

Copy link
Copy Markdown

CI triage

CI failed on this PR. Automated classification of the 2 failed job(s):

  • Unit coverageneeds investigation: inspect the failing step and uploaded diagnostics; rerun only after classifying the cause.
  • PR requiredneeds investigation: inspect the failing step and uploaded diagnostics; rerun only after classifying the cause.

Compared with main CI run #3245 (success).

Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger.

@BigSimmo

Copy link
Copy Markdown
Owner Author

Superseded by merged PR #842, which already contains the same design-audit service/test updates and is now on main.

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.

1 participant