test(e2e): add a Playwright harness for the Ask Agent capabilities - #421
Merged
Conversation
No Playwright config existed despite the package already being a devDependency. Add playwright.config.ts (points at the running docker-compose stack, not a Playwright-managed dev server -- the app needs Postgres/Keycloak/Valkey/orchestrator alongside it), a real Keycloak-login helper (drives the actual OIDC redirect form with the synthetic demo.analyst credentials, not a token injected into storage), a validated smoke spec, and ask-agent.spec.ts covering all four Ask Agent capabilities (relative-time retrieval #415, multi-lineage graphs #418, image citation #419, Layer Popup #420). The login flow is verified passing against a live stack right now. ask-agent.spec.ts needs #415/#418/#419/#420 merged and the images rebuilt from main before it can pass -- verified during development that the currently-running ad-hoc stack is built from an unrelated, more advanced branch (its own conversation-history UI), not main, so it cannot validate this spec; that requires a proper CI/deployment rebuild, out of this checkpoint's scope. Part of the Ask Agent temporal/lineage/evidence goal (checkpoint 5 of 6 -- e2e harness).
|
Warning Review limit reached
Next review available in: 10 minutes Limit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (11)
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. Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
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
enabled auto-merge (squash)
August 23, 2026 06:41
…k' into repair/pr421-frontend-root-3
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
seonghobae
enabled auto-merge (squash)
August 23, 2026 07:07
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
seonghobae
enabled auto-merge (squash)
August 23, 2026 07:40
…assertions The git-branch-lineage and image-citation Ask Agent tests wrapped their only real assertions in an `if (count > 0)` guard, so a run where the LLM's answer happened not to produce that citation shape would report PASS without exercising the behavior the test claims to cover. Assert the precondition itself so the test fails loudly instead. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B5M79L945DMyMs3sg5yJ14
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Contributor
Author
|
Fixed the exact-head E2E review findings in |
seonghobae
enabled auto-merge (squash)
August 23, 2026 08:24
seonghobae
added a commit
that referenced
this pull request
Aug 24, 2026
) * docs: record ADRs for the Ask Agent temporal/lineage/evidence goal Four new ADRs, one per checkpoint of the Ask Agent temporal/lineage/ evidence goal: - 0119: Korean relative-time expression resolution (#415) - 0120: multi-thread Event Lineage graphs in Ask answers (#418) - 0121: image citation without a new image-serving surface (#419) - 0122: the evidence Layer Popup (#420) Update CHANGELOG.md's Unreleased section and add an "Ask Agent Gaps" section to docs/product-technical-gap-baseline.md marking all four gaps (plus e2e coverage, #421) resolved, following that file's existing "(Resolved)" convention. Part of the Ask Agent temporal/lineage/evidence goal (checkpoint 6 of 6 -- documentation). * docs: renumber Ask Agent ADRs 0119-0122 to 0150-0153 Cross-session coordination surfaced a widespread ADR-numbering collision: at least ten numbers between 0119 and 0143 are independently claimed by concurrent unmerged branches across other sessions (0127/0128/0129/0131/0132 each claimed 2-4x, per `git log --all --diff-filter=A -- docs/adr`). This PR's own 0119-0122 was a three-way collision (also claimed by the TEPP topic-lineage PR and a quantity-superscript PR). Since this PR only holds 4 ADRs against another's 14 (0119-0132), renumbering here is the smaller diff. Moved clear of every number seen across all branches (highest observed: 0143), leaving buffer room. No content changes -- only the ADR number in each file's title, their mutual cross-references, and every CHANGELOG.md / gap-baseline.md citation of the old numbers. * docs: refresh product technical gap baseline * docs: record armed acceptance queue * fix(frontend): repair the inherited login/admin-panel build break 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. --------- Co-authored-by: seonghobae <seonghobae@users.noreply.github.com>
seonghobae
added a commit
that referenced
this pull request
Aug 24, 2026
…-source-detail-state-filter origin/main moved 58 commits ahead mid-session, including a properly reviewed and tested implementation of the same evidence-layer-popup feature this branch had manually rebuilt (#419). Re-merged to pick it up rather than ship a parallel, untested version. Resolved 7 conflicts: - backend/app/post_chat_ingestion.py: combined this branch's ABAC/candidate-id filtering with main's Korean relative-time date-range filter on the same source_post query; combined both sides' import additions (tepp_client + temporal_expressions). - backend/app/lineage_ingestion.py: combined this branch's isolation_reason (ADR 0143) with main's include_isolated parameter and new lineage_graphs_for_posts merge function -- both needed together, not alternatives. - backend/app/main.py: added the lineage_graph computation (from main's dropped PR #418) into this branch's persist_turn-based ask_agent flow, using response["cited_post_ids"] so it works for both the sources-empty and populated-sources paths. - frontend/src/App.tsx: kept this branch's working multi-turn (exchanges.map()) Ask Agent implementation wholesale over main's incompatible pre-refactor single-answer fragment; added the missing <LineageDag> rendering for response.lineage_graph per exchange, and kept the richer TenantConfig AdminPanel wiring already established. - tests/test_lineage_ingestion.py, tests/test_global_ask_sources.py, CHANGELOG.md: reconstructed interleaved additive conflicts as complete, non-overlapping test functions / changelog entries from both sides. Also fixed real bugs surfaced along the way: - cited_post_images() (backend/app/post_chat_ingestion.py) queried the pre-rename image.caption column; migration 0104 renamed it to image_caption. Fixed the SQL and row mapping, matching how /api/posts/{id}/content already does it. - Two tenant-settings tests asserted the old single-brandName response shape instead of the current {brandName, systemName, copyrightYear, copyrightHolder} contract. - A stale test fixture in tests/test_post_chat_ingestion.py still keyed its fake DB rows by the pre-rename "caption" column. - Fixed a duplicate ADR 0119 number (retire-buyer-terminology -> 0168; leftover-map-two-dimensional-distance already owned 0119). - A LineageDag.test.tsx role="group" vs a newly-merged App.test.tsx role="img" mismatch: kept role="group" (this branch's existing, better-tested LineageDag.tsx choice, verified by 4 passing assertions) and updated the one new test instead. Verified: tests/ (1037 passed, 11 skipped), backend/tests/ (156 passed, 5 skipped -- orchestrator-gated), frontend (469 passed), tsc -b clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NpWw9SnPBaemFZmW3fdTVM
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Buyer-visible gap
No Playwright config existed anywhere in the repo despite
playwrightalready being a frontend devDependency -- the four Ask Agent
capabilities in this goal (and every other UI feature) had no
end-to-end coverage against a real running app.
Change
playwright.config.ts: points at the already-runningdocker composestack (
LINEAGEWEAVE_E2E_BASE_URL, defaults tohttp://localhost:15173)rather than a Playwright-managed dev server -- the app needs
Postgres/Keycloak/Valkey/the orchestrator alongside it, which a
webServerblock can't provide.e2e/support/auth.ts: logs in through the real Keycloak-hosted OIDClogin form with the synthetic
demo.analystcredentials (the sameones
backend/tests/test_api.py's_fetch_demo_analyst_tokenuses) --not a token injected into storage, so the suite exercises the actual
authorization-code redirect flow.
e2e/smoke.spec.ts: verified passing right now against the livestack (login -> authenticated destination visible).
e2e/ask-agent.spec.ts: covers all four Ask Agent capabilities --relative-time-scoped retrieval (feat(ask): resolve Korean relative-time expressions in Global Ask #415), git-branch-style multi-lineage
rendering (feat(ask): return a merged lineage_graph for every cited post #418), image citation (feat(ask): cite persisted image evidence for cited posts #419), and the evidence Layer Popup
(feat(ask): show cited-post evidence in a Layer Popup #420).
tsconfig.e2e.json(+ registered intsconfig.json) soe2e/andplaywright.config.tstype-check as part oftsc -b, matching theexisting
tsconfig.app.json/tsconfig.node.jsonsplit.pnpm run e2escript;test-results/playwright-reportadded to.gitignore.Why
ask-agent.spec.tsisn't passing yetVerified during development: the
docker composestack currentlyrunning locally was built from an unrelated, more advanced branch with
its own conversation-history Ask Agent UI -- not
main. That's a factabout this dev environment, not a bug in the spec.
ask-agent.spec.tsis written against
main+ #415/#418/#419/#420 and needs all fourmerged and the images rebuilt before it can pass; wiring that rebuild
into CI is a separate follow-up, out of this checkpoint's scope.
Verification
pnpm exec playwright test e2e/smoke.spec.ts-- 1 passed, against thelive stack, right now.
pnpm run lint/tsc -b-- no new warnings/errors from the e2efiles; the two pre-existing
App.tsxerrors are the unrelatedmainbreak already tracked against feat: show persisted image-region locations (v2.12.8) #405.
uv run --frozen python -m pytest -q-- 753 passed, 17 skipped(backend untouched by this PR; ran for regression safety).
No real records, identifiers, or provider credentials are included --
demo.analyst/lineageweave-demo-onlyare synthetic seed credentialsshared with the existing backend integration test suite.