Skip to content

Harden document-mode smoke switching follow-up#80

Merged
BigSimmo merged 8 commits into
mainfrom
copilot/fix-unresolved-pr-review-comments
Jun 27, 2026
Merged

Harden document-mode smoke switching follow-up#80
BigSimmo merged 8 commits into
mainfrom
copilot/fix-unresolved-pr-review-comments

Conversation

Copilot AI commented Jun 27, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Address the remaining PR [codex] Update clinical guide for Node 24 #78 review follow-up by making document-mode smoke coverage fail fast when no supported mode switch control is present.
  • Keep compatibility with both the legacy mode toggle and the newer app-mode menu path without widening production behavior.

Verification

  • npm run verify:cheap
  • npm run verify:ui when UI, routing, styling, browser behavior, reduced-motion, or forced-colors behavior changed
  • npm run verify:release before release or handoff confidence claims
  • npm run format:check
  • npm run check:production-readiness when clinical workflow, privacy, environment, Supabase, source governance, or deployment behavior changed

Clinical Governance Preflight

Complete this section when the change touches ingestion, answer generation, search/ranking, source rendering, document access, privacy, production env, or clinical output.

  • 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

Notes

  • Smoke helper hardening

    • preserve both supported mode-switch paths
    • fail immediately with a clear error when neither control is available
    • tighten menu-item selection and reuse locators to reduce ambiguity
  • Scope

    • follow-up is limited to tests/ui-smoke.spec.ts
    • DocumentViewer.tsx no longer carries the stale DocumentBadge import
async function switchToDocumentSearchMode(page: Page) {
  const legacyDocumentsMode = page.getByRole("button", { name: "Switch to document search mode" });
  if (await isVisibleWithoutThrow(legacyDocumentsMode)) return useLegacyToggle();

  const appModeMenu = page.getByRole("button", { name: /^Current app mode:/ });
  if (!(await isVisibleWithoutThrow(appModeMenu))) {
    throw new Error(
      "Could not switch to document search mode: neither the legacy mode toggle nor the app mode menu is visible.",
    );
  }

  return useModeMenu();
}

Copilot AI changed the title [WIP] Fix unresolved PR review comments from PR #78 Harden document-mode smoke switching follow-up Jun 27, 2026
Copilot AI requested a review from BigSimmo June 27, 2026 09:40
@BigSimmo
BigSimmo marked this pull request as ready for review June 27, 2026 09:54
Copilot AI review requested due to automatic review settings June 27, 2026 09:54
@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.

@BigSimmo
BigSimmo merged commit dcdc200 into main Jun 27, 2026
5 checks passed

Copilot AI 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.

Pull request overview

This PR hardens the Playwright UI smoke helper that switches the dashboard into “Documents” mode by supporting both the legacy toggle and a newer “current app mode” menu path, and failing fast with a clear error when neither control is available.

Changes:

  • Add a small isVisibleWithoutThrow() helper to probe locator visibility without strict-mode exceptions aborting the flow.
  • Update switchToDocumentSearchMode() to (1) use the legacy toggle when present, otherwise (2) fall back to the app-mode menu, and (3) throw a clear error when neither control is visible.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/ui-smoke.spec.ts
Comment on lines +56 to +60
await expect(legacyDocumentsMode).toBeEnabled();
await expect(async () => {
await legacyDocumentsMode.click();
await expect(legacyDocumentsMode).toHaveAttribute("aria-pressed", "true", { timeout: 1_000 });
}).toPass({ timeout: 8_000 });
Comment thread tests/ui-smoke.spec.ts
Comment on lines 72 to 79
await expect(async () => {
await appModeMenu.click();
await expect(appModeMenu).toHaveAttribute("aria-expanded", "true", { timeout: 2_000 });
const documentsMode = page.getByRole("menuitemradio", { name: /^Documents\b/ });
await expect(documentsMode).toBeVisible({ timeout: 3_000 });
await documentsMode.click();
await expect(documentsMode).toHaveAttribute("aria-pressed", "true", { timeout: 1_000 });
await expect(appModeMenu).toHaveAccessibleName("Current app mode: Documents", { timeout: 2_000 });
}).toPass({ timeout: 8_000 });
@BigSimmo
BigSimmo deleted the copilot/fix-unresolved-pr-review-comments branch June 28, 2026 01:47
cursor Bot pushed a commit that referenced this pull request Jul 25, 2026
…mplate cleanup (#1204)

* fix(#30): match expected files by distinct document identity

The merged #30 fix assigned each expectation to a distinct source
position. Positions are not documents: answer.citations carries one
entry per cited chunk, so a single combo-titled document arrives
several times and could still fill both comparison slots. First-come
assignment was also order-dependent -- a document matching two
expectations could consume the only source a narrower expectation
needed, so the same inputs produced different allHit results depending
on expectedFiles order.

Dedupe the window by document text and assign sources by maximum
bipartite matching, so coverage is both distinct-by-document and
order-independent.

RAG impact: no retrieval behaviour change -- eval matching only.

Also append label rows in place when building labelsByDocument; paging
past the 1,000-row cap (#75) means that loop now sees the full label
set, where per-row array rebuilds were quadratic.

Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>

* chore: remove leftover PR_POLICY_BODY.md after #1153

The sync-pr-policy-body CI job overwrites a pull request description
with this file whenever it exists on the head commit. The copy on main
holds the audit-remediation body from #1153, so every branch cut from
main silently inherits that description -- which is how #1177 lost its
own body. Remove it, as #953 did after #939; the job skips cleanly when
no template is present.

Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>

* docs(issues): record the #30 follow-up and open #80

Drop the stale queue row for #30/#75, which stayed in the recommended
queue after both were archived, note the distinct-identity hardening on
the #30 archive row, and open #80 to re-test the removed
admission-to-discharge widening under approved eval evidence.

Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>

* docs(ledger): append #1177 SHA correction and this review record

Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>

---------

Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
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.

3 participants