Skip to content

test(api): cover Ask, Rankings, and locale preference boundaries - #435

Merged
seonghobae merged 9 commits into
worktree-fix-frontend-build-breakfrom
worktree-fix-untested-endpoints
Aug 23, 2026
Merged

test(api): cover Ask, Rankings, and locale preference boundaries#435
seonghobae merged 9 commits into
worktree-fix-frontend-build-breakfrom
worktree-fix-untested-endpoints

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Exercises buyer-facing POST /api/ask, GET /api/rankings, and PATCH /api/me/preferences through the real FastAPI/PostgreSQL/Keycloak integration boundary.
  • Replaces the nondeterministic ranking assertion with exact fail-closed and deterministic accepted outcomes, proving only ABAC-visible synthetic posts and stored titles enter RankWeave channels.
  • Restores the saved locale after the persistence test with a closed psycopg2 connection, and proves invalid or anonymous writes cannot change it.
  • Uses FastAPI's current 422 alias on the newly covered empty Ask path without changing the wire status.
  • Keeps Starlette TestClient on the project's official httpx dev dependency; the removed look-alike httpx2 package and its stale documentation are absent.
  • Keeps the branch dependency-correct: the duplicate App/OIDC/Admin diff owned by fix: repair frontend build and publish ontology Pages #426 is absent.

Exact-head verification

Validated at head 7dd6ce01e3c14ea1972554634b0026a3ccf0200b stacked on exact #426 base a44f360749b98c9471bfedbfce9656a41e729214:

  • Targeted Ask/Rankings/locale integration run — 9 passed, 103 deselected.
  • Documentation hygiene — 4 passed.
  • uv lock --check / dependency graph remains on official httpx; no httpx2 branch diff.
  • git diff --check.
  • Hosted exact-head checks and independent approval are still required.
  • Effective stacked diff contains only ARCHITECTURE.md, the API changelog fragment, backend/app/main.py, and backend/tests/test_api.py; no App/OIDC/Admin file remains.
  • fix: repair frontend build and publish ontology Pages #426 must merge first. This PR must then be retargeted to protected main and receive fresh exact-head/base checks and independent approval.

The unchanged product-code test head immediately before the documentation-only provenance correction also passed the full hosted Python suite. That prior-head result is not treated as an exact-head merge gate; the fresh hosted run must complete.

Governance

  • Synthetic fixtures only; no source-organization records, identifiers, or credentials.
  • RankWeave remains the sole ranking/fusion owner under ADR 0024. The injected adapter makes the HTTP contract deterministic; it does not reimplement fusion or invent theta.
  • Auto-merge is disabled while the base is an unprotected stack branch. Merge remains protected until fix: repair frontend build and publish ontology Pages #426 lands, this PR returns to main, and terminal exact-head/base required checks plus an independent exact-head approval are present.
Open in Devin Review

…erences

Found via a systematic route-vs-test cross-reference (every @app.get/
post/patch/put/delete path in backend/app/main.py checked against
every test file, not just backend/tests/test_api.py) -- same technique
that found the /healthz routing bug earlier this session. All three
endpoints had zero test coverage anywhere in the repo:

- POST /api/ask: the Ask Agent endpoint itself was never exercised at
  the HTTP layer, despite its underlying functions
  (gather_global_chat_sources, cited_post_evidence, ...) being
  unit-tested. New tests cover the empty-question 422, the
  no-orchestrator-configured 503 (Null client, matching the existing
  derive-commitment 503 test's monkeypatch pattern), and the
  unauthenticated 401/403 case.
- GET /api/rankings: covers the real response contract
  (RankWeave's own "never invent a fused score" fail-closed shape --
  status is either "accepted" or "unavailable", never a guessed
  ranking) plus the unauthenticated case.
- PATCH /api/me/preferences: covers persisting a supported locale
  (round-tripped through GET /api/me) and rejecting an unsupported one
  (Pydantic's own Literal validation, previously untested).

uv run --frozen python -m pytest -q: 760 passed, 17 skipped.
@coderabbitai

coderabbitai Bot commented Aug 22, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 630627eb-b0bc-4c8c-b14d-81cc3f90dae0

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Open in Devin Review

Same shared-ancestor bug as #418/#415/#426/#427/#429/#431/#434/#436:
the login button built an unsanitized returnUrl inline instead of
returnUrlFromLocation()/rememberOidcReturnUrl(), and removed the
unreachable login-screen AdminPanel render (accessToken is always
undefined pre-auth).
@seonghobae
seonghobae enabled auto-merge (squash) August 22, 2026 12:26
devin-ai-integration[bot]

This comment was marked as resolved.

seonghobae added a commit that referenced this pull request Aug 22, 2026
Same shared-ancestor bug as #418/#415/#426/#427/#429/#431/#434/#435/#436:
the login button built an unsanitized returnUrl inline instead of
returnUrlFromLocation()/rememberOidcReturnUrl(), and removed the
unreachable login-screen AdminPanel render (accessToken is always
undefined pre-auth). This PR's own diff doesn't touch AdminPanel.
seonghobae added a commit that referenced this pull request Aug 22, 2026
Same shared-ancestor bug as #418/#415/#426/#427/#429/#431/#434/#435/#436/#437:
the login button built an unsanitized returnUrl inline instead of
returnUrlFromLocation()/rememberOidcReturnUrl(), and removed the
unreachable login-screen AdminPanel render (accessToken is always
undefined pre-auth). This PR's own diff doesn't touch AdminPanel.
@seonghobae

Copy link
Copy Markdown
Contributor Author

Confirmed — no functional application changes, just replacing the raw returnUrl with the existing safe helpers and dropping unreachable dead code, matching #426's approach.

@seonghobae

Copy link
Copy Markdown
Contributor Author

Confirmed, thanks — no action needed.

seonghobae added a commit that referenced this pull request Aug 22, 2026
Same shared-ancestor bug as #418/#415/#426/#427/#429/#431/#434/#435/#436/#437/#438:
the login button built an unsanitized returnUrl inline instead of
returnUrlFromLocation()/rememberOidcReturnUrl(), and removed the
unreachable login-screen AdminPanel render.
seonghobae added a commit that referenced this pull request Aug 22, 2026
Same shared-ancestor bug as #418/#415/#426/#427/#429/#431/#434/#435/#436/#437/#438/#439:
the login button built an unsanitized returnUrl inline instead of
returnUrlFromLocation()/rememberOidcReturnUrl(), and removed the
unreachable login-screen AdminPanel render.
@seonghobae seonghobae changed the title test(api): cover POST /api/ask, GET /api/rankings, PATCH /api/me/preferences test(api): cover POST /api/ask, GET /api/rankings, PATCH /api/me/preferences; fix(frontend): wire OIDC return-url on login Aug 23, 2026
seonghobae added a commit that referenced this pull request Aug 23, 2026
* test(frontend): add Storybook coverage for BuyerNav

BuyerNav had a test file but no story, the last remaining gap in
frontend/src/components/*.tsx test+story coverage. Adds stories for
each destination plus an edge case with an extra tools slot.

* fix(frontend): use OIDC return-url helpers on the login button

Same shared-ancestor bug as #418/#415/#426/#427/#429/#431/#434/#435/#436/#437:
the login button built an unsanitized returnUrl inline instead of
returnUrlFromLocation()/rememberOidcReturnUrl(), and removed the
unreachable login-screen AdminPanel render (accessToken is always
undefined pre-auth). This PR's own diff doesn't touch AdminPanel.

* test(frontend): cover WorkspaceNav in Storybook
@seonghobae

Copy link
Copy Markdown
Contributor Author

Exact-head review requested for d63b8ebf42065e18da84d49d78b8c9d0c7128e62.

Please review the effective origin/main...HEAD diff, especially:

  1. deterministic accepted/unavailable RankWeave HTTP contracts and exclusion of the synthetic other-entity private post;
  2. account locale state restoration and invalid/anonymous write boundaries;
  3. the dev-only httpx2 dependency against Starlette's installed package contract and its resolved uv.lock entries;
  4. the deliberate removal of all frontend/OIDC effective changes, which remain owned by protected dependency fix: repair frontend build and publish ontology Pages #426.

All prior review threads are resolved and outdated. Do not transfer approval from an earlier head.

devin-ai-integration[bot]

This comment was marked as resolved.

Remove stale httpx2 claims after the look-alike dependency was deleted; the harness uses Starlette TestClient with the official httpx dev dependency.
Two TypeScript build errors on main (blocking every open PR's
"Frontend lint, test, build" check, including this repo's own review
bot's ability to approve them):

- App.tsx imported rememberOidcReturnUrl/returnUrlFromLocation from
  oidcReturnUrl.ts but never called them -- the login button built its
  own unsanitized returnUrl inline instead of using the safe helper
  (oidcReturnUrl.ts's isSafeReturnUrl guard against an open-redirect-
  shaped value) or persisting it as the sessionStorage/localStorage
  fallback restoreOidcReturnUrl (already wired up on the callback side
  in main.tsx) reads when the OIDC state round-trip drops it.
- The unauthenticated login screen unconditionally rendered
  <AdminPanel accessToken={accessToken} /> when destination === "admin"
  -- accessToken is string | undefined here (always undefined while
  unauthenticated), a real type error, and the render was unreachable
  through normal navigation (destination only changes via the
  authenticated nav) -- dead code, removed.

uv run --frozen python -m pytest -q: 753 passed, 17 skipped.
pnpm run test: 140 passed. pnpm run lint / build: clean.
@seonghobae seonghobae changed the title test(api): cover POST /api/ask, GET /api/rankings, PATCH /api/me/preferences; fix(frontend): wire OIDC return-url on login test(api): cover Ask, Rankings, and locale preference boundaries Aug 23, 2026
@seonghobae

Copy link
Copy Markdown
Contributor Author

Exact-head review requested for 2bc33220. The only follow-up from 0b8374b5 corrects stale architecture/changelog claims after the look-alike httpx2 dependency was removed; the branch uses official httpx, retains the 9-test buyer-route contract, excludes #426-owned App changes, and remains protected pending hosted gates and independent approval.

@seonghobae
seonghobae changed the base branch from main to worktree-fix-frontend-build-break August 23, 2026 16:12
@seonghobae
seonghobae disabled auto-merge August 23, 2026 16:12
@seonghobae

Copy link
Copy Markdown
Contributor Author

Stack boundary corrected: exact head 7dd6ce01 is now based on #426 branch worktree-fix-frontend-build-break at a44f3607. The effective PR diff is limited to the four API-contract files; the concurrent duplicate App/OIDC/Admin commit was reverted because #426 carries the stronger shared implementation/tests. Auto-merge is disabled on this unprotected stack base. After #426 merges, retarget this PR to protected main and re-establish all exact-head/base checks and independent approval.

seonghobae added a commit that referenced this pull request Aug 23, 2026
* test(frontend): cover LineageDag with tests and stories

LineageDag renders the git-branch-style multi-thread lineage graph
used by both the post-detail popup and the Ask Agent's multi-lineage
answer view (ADR 0120), and had zero test or story coverage despite
being a core, non-trivial component. Adds tests for the empty state,
multi-group branch rendering, group-heading fallback for missing/UUID
groups, click and keyboard node selection, the current-post marker,
and label truncation with an accessible full-label fallback. Adds
stories for empty, single-branch, multi-branch (with an actual fork),
ungrouped, and long-label scenarios.

* fix(frontend): use OIDC return-url helpers on the login button

Same shared-ancestor bug as #418/#415/#426/#427/#429/#431/#434/#435/#436/#437/#438:
the login button built an unsanitized returnUrl inline instead of
returnUrlFromLocation()/rememberOidcReturnUrl(), and removed the
unreachable login-screen AdminPanel render.

* Revert "fix(frontend): use OIDC return-url helpers on the login button"

This reverts commit 590c6c3.

* fix(frontend): keep lineage edge evidence visible-only

* fix(frontend): terminate rooted lineage cycles

* chore(frontend): keep shared OIDC repair on #426

* refactor(frontend): remove unreachable cycle guard

* test(frontend): cover converging lineage DAGs

* fix(frontend): position converging DAG nodes once
seonghobae added a commit that referenced this pull request Aug 23, 2026
* test(frontend): cover FiveW1H component with tests and stories

Adds Vitest coverage for loading state, empty-evidence messaging,
raw-source-to-label mapping (including the unmapped fallback), and
optional evidence-text/ontology-badge rendering, plus a Storybook
inventory covering the loading, all-empty, grounded-answer, and
unmapped-source scenarios.

* fix(frontend): use OIDC return-url helpers on the login button

Same shared-ancestor bug as #418/#415/#426/#427/#429/#431/#434/#435/#436:
the login button built an unsanitized returnUrl inline instead of
returnUrlFromLocation()/rememberOidcReturnUrl(), and removed the
unreachable login-screen AdminPanel render (accessToken is always
undefined pre-auth). This PR's own diff doesn't touch AdminPanel.

* chore(frontend): keep FiveW1H coverage dependency-correct

Remove the duplicated OIDC/login changes owned by #426 so this PR carries only its FiveW1H tests and Storybook inventory.
@seonghobae
seonghobae merged commit b2d9bdb into worktree-fix-frontend-build-break Aug 23, 2026
4 checks passed
seonghobae added a commit that referenced this pull request Aug 23, 2026
* test(admin): cover AdminPanel (previously zero coverage)

Found via a systematic component-vs-story-vs-test cross-reference of
frontend/src/components/*.tsx -- AdminPanel had neither a .test.tsx
nor a .stories.tsx, unlike every other component in the directory.

Tests cover: save disabled until the brand name actually changes,
a successful save calling updateTenantConfig (backend/app/main.py's
PATCH /api/settings, now covered separately in #435) with the right
arguments and reporting the new name back to the caller, and a failed
save showing the error while leaving the form editable (not stuck
disabled). Storybook stories cover the default state and a long
brand-name layout edge case.

pnpm run test: 143 passed (was 140). pnpm run lint: no new warnings.
pnpm run build: the two pre-existing App.tsx errors are the unrelated
main break already fixed in #426, not something this PR touches.

* fix(frontend): use OIDC return-url helpers on the login button

Same shared-ancestor bug as #418/#415/#426/#427/#429/#431/#434: the
login button built an unsanitized returnUrl inline instead of
returnUrlFromLocation()/rememberOidcReturnUrl(), and removed the
unreachable login-screen AdminPanel render (accessToken is always
undefined pre-auth). This PR's own AdminPanel.test.tsx/.stories.tsx
render the component directly, so this doesn't affect its coverage.

* fix(admin): guard no-op saves and announce results

* docs(storybook): inventory tenant settings
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