qa-lab: add evidence artifact gallery#94283
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a QA Lab “Evidence” gallery that can load existing qa-evidence.json bundles, summarize entries/artifacts/coverage, and serve declared artifacts via constrained server routes for safe inspection in the QA Lab web UI.
Changes:
- Add server-side evidence gallery model + artifact resolution constrained to declared evidence artifacts.
- Add
/api/evidenceand/api/evidence/artifactendpoints to load evidence and stream artifacts. - Add a new “Evidence” tab UI (filters, entry drilldown, artifact previews/links) plus styling and render tests.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| extensions/qa-lab/src/evidence-gallery.ts | Builds evidence gallery DTOs, resolves evidence + declared artifacts, and derives optional producer context. |
| extensions/qa-lab/src/evidence-gallery.test.ts | Coverage for evidence parsing, declared artifact gating, and containment behaviors. |
| extensions/qa-lab/src/lab-server.ts | Adds Evidence API routes and artifact streaming. |
| extensions/qa-lab/src/lab-server.test.ts | Adds tests for evidence artifact HEAD/GET behavior. |
| extensions/qa-lab/web/src/app.ts | Adds client-side evidence state, URL handling, and evidence load actions. |
| extensions/qa-lab/web/src/ui-render.ts | Adds Evidence tab rendering (filters, entry list, artifact cards, producer context). |
| extensions/qa-lab/web/src/ui-render.test.ts | Render tests for evidence statuses and UX Matrix linkage behavior. |
| extensions/qa-lab/web/src/styles.css | Evidence tab layout/styles and evidence-focus shell styling. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 142f7b234d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Codex review: needs maintainer review before merge. Reviewed June 18, 2026, 10:15 AM ET / 14:15 UTC. Summary PR surface: Source +2177, Tests +890, Other 0. Total +3067 across 10 files. Reproducibility: not applicable. as a bug reproduction because this is a feature PR. The PR body includes after-change live browser output showing Review metrics: 1 noteworthy metric.
Stored data model Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Mantis proof suggestion Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land this gallery slice after maintainers accept the declared-artifact local file-serving boundary and current-head checks finish, while keeping the UX Matrix scenario and workflow wiring in their separate follow-up PRs. Do we have a high-confidence way to reproduce the issue? Not applicable as a bug reproduction because this is a feature PR. The PR body includes after-change live browser output showing Is this the best way to solve the issue? Yes, this is the best scoped solution for the gallery slice: it stays inside the QA Lab plugin and reuses existing AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 55750f7c6dd6. Label changesLabel justifications:
Evidence reviewedPR surface: Source +2177, Tests +890, Other 0. Total +3067 across 10 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
Review: evidence gallery — maintainability + IA notesNice, well-scoped feature and the path-containment design is solid (realpath + Maintainability
Security (design is sound; test gaps)
Lossless diff reduction (~150+ LOC, no behavior change)
Information architecture: Results / Report / Evidence (conceptual, not data)The three tabs are genuinely different data sources, but to a user they all read as "the outcome of the run," and the tab names don't signal the distinction that actually matters. Today the split is along an engineering axis (data source/format) rather than a user-task axis:
This PR introduces the third overlapping name, so it's a good moment to at least disambiguate. Options, cheap → ambitious:
Suggest doing D here (label/subtitle only) and filing B/C as the IA cleanup alongside PR 3/4. Not a merge blocker. |
62179b6 to
c0cfa19
Compare
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
@clawsweeper re-review Applied the recommended no-store fix for Proof run locally on
|
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
…nder/test duplication
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
* qa-lab: add evidence artifact gallery * qa-lab: harden evidence gallery artifacts * qa-lab: share evidence gallery view types * qa-lab: disable evidence preview caching * refactor(qa-lab): resolve evidence artifacts once and trim gallery render/test duplication * fix(qa-lab): build evidence model before sending /api/evidence success headers --------- Co-authored-by: Dallin Romney <[email protected]>
* qa-lab: add evidence artifact gallery * qa-lab: harden evidence gallery artifacts * qa-lab: share evidence gallery view types * qa-lab: disable evidence preview caching * refactor(qa-lab): resolve evidence artifacts once and trim gallery render/test duplication * fix(qa-lab): build evidence model before sending /api/evidence success headers --------- Co-authored-by: Dallin Romney <[email protected]>
Summary
This PR adds a QA Lab evidence gallery for existing
qa-evidence.jsonoutputs./api/evidenceand/api/evidence/artifactroutes so the QA Lab web app can inspect artifacts without exposing arbitrary repo files.no-store/nosniff, and undeclared/outside artifacts return controlled client errors.Success looks like maintainers can open a QA Lab evidence artifact from a suite run and inspect screenshots, videos, logs, json validation output, coverage IDs, and run context from the web app.
Reviewers should focus on path containment/artifact authorization, the evidence model shape, and whether the UI is generic enough for QA Lab evidence beyond the UX matrix producer.
Linked context
Related #94276
Related #94273
This split follows maintainer feedback to pull the original combined QA Lab / UX matrix work apart:
Real behavior proof (required for external PRs)
qa-evidence.jsondirectly.codex/qa-lab-evidence-gallery-main, headc0cfa19422.node --import tsx -e "import fs from 'node:fs/promises'; import path from 'node:path'; import { chromium } from 'playwright'; import { startQaLabServer } from './extensions/qa-lab/src/lab-server.ts'; const repoRoot = process.cwd(); const evidencePath = '.artifacts/ux-matrix/20260617T203150Z-8ac4664d16/qa-evidence.json'; const proofDir = path.join(repoRoot, '.artifacts', 'qa-lab-evidence-gallery-proof', '20260617T2258Z-142f7b234d'); await fs.mkdir(proofDir, { recursive: true }); const server = await startQaLabServer({ repoRoot, host: '127.0.0.1', port: 0, embeddedGateway: 'disabled' }); let browser; try { browser = await chromium.launch({ headless: true }); const context = await browser.newContext({ viewport: { width: 1440, height: 1050 }, recordVideo: { dir: proofDir, size: { width: 1440, height: 1050 } } }); const page = await context.newPage(); const url = server.baseUrl + '/evidence?path=' + encodeURIComponent(evidencePath); await page.goto(url, { waitUntil: 'networkidle' }); await page.getByText('Evidence Archive').first().waitFor({ state: 'visible', timeout: 10000 }); await page.locator('.evidence-entry-card').first().waitFor({ state: 'visible', timeout: 10000 }); await page.locator('.evidence-artifact-card').first().waitFor({ state: 'visible', timeout: 10000 }); const screenshotPath = path.join(proofDir, 'evidence-archive-loaded.png'); await page.screenshot({ path: screenshotPath, fullPage: true }); const entryCount = await page.locator('.evidence-entry-card').count(); const artifactCount = await page.locator('.evidence-artifact-card').count(); const matrixCellCount = await page.locator('.evidence-matrix-cell').count(); const title = await page.title(); await context.close(); const videos = (await fs.readdir(proofDir)).filter((file) => file.endsWith('.webm')); console.log(JSON.stringify({ baseUrl: server.baseUrl, url, title, evidencePath, screenshotPath, videos: videos.map((file) => path.join(proofDir, file)), entryCount, artifactCount, matrixCellCount }, null, 2)); } finally { if (browser) await browser.close().catch(() => {}); await server.stop(); }"qa-evidence.json, rendered evidence entries, artifact cards, producer context, a scorecard preview, artifact links, and the UX matrix mini-grid.Tests and validation
Commands run:
pnpm run lint:tmp:no-raw-channel-fetch node scripts/run-vitest.mjs extensions/qa-lab/src/evidence-gallery.test.ts extensions/qa-lab/src/lab-server.test.ts extensions/qa-lab/web/src/ui-render.test.ts pnpm qa:lab:build node scripts/run-tsgo.mjs -p tsconfig.extensions.json --incremental false pnpm lint --threads=8 git diff --check origin/main...HEADRegression coverage added:
HEAD/GET,no-store/nosniffresponse headers, undeclared artifacts, and outside artifact requests.Risk checklist
Did user-visible behavior change?
YesDid config, environment, or migration behavior change?
NoDid security, auth, secrets, network, or tool execution behavior change?
YesHighest-risk area: serving local artifact files through the QA Lab server.
Mitigation: artifact downloads are constrained to declared artifacts from the selected
qa-evidence.json, resolved inside the repo/evidence roots, served withno-storeandnosniff, and covered by path containment/negative HTTP tests.Current review state
Next action: CI rerun and ClawSweeper/reviewer re-review.
Waiting on: GitHub CI and reviewer feedback after
c0cfa19422.Bot/reviewer comments addressed: raw fetch boundary line drift, artifact response hardening, missing bundle-local artifact fallback, malformed matrix cell parsing, controlled API status mapping, canonical evidence path URLs,
noopener noreferrerartifact links, producer-file manifest duplication, sharedtoRepoRelativePath, bounded artifact view concurrency, shared Evidence DTO type aliases, and cheap Evidence Archive IA label/subtitle.