test(shared): add golden transcript fixtures#401
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdds shared golden mixed-source transcript fixtures, Vitest coverage for transcript ordering and adapter output, an e2e chat-flow contract update to consume the fixtures, and a progress-tracker note for T2.1. ChangesGolden Mixed-Source Transcript Fixtures
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
app/web/src/__e2e__/chat-flow-contract.spec.ts (1)
49-50: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDerive the asserted transcript text from the shared fixture instead of duplicating literals.
The spec already imports the golden message/event fixtures, but it still hard-codes the kickoff and replay-summary text in three places. Pull those expected strings from the shared fixtures once so wording changes only land in one source of truth. As per coding guidelines,
**/*.{ts,tsx,js,jsx}: Do not write tests that merely replicate implementation branches, assert constant strings, hard-code error text as behavior, or mock the function under test itself.Also applies to: 70-73, 108-108
🤖 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 `@app/web/src/__e2e__/chat-flow-contract.spec.ts` around lines 49 - 50, The transcript assertions in the chat-flow contract spec are hard-coding kickoff and replay-summary text instead of deriving them from the shared fixtures. Update the assertions in the relevant test blocks to read the expected strings from the imported golden message/event fixtures once, and reuse those fixture-derived values everywhere the text is checked. Use the existing fixture imports in chat-flow-contract.spec.ts as the single source of truth so wording changes only need to be made in one place.Source: Coding guidelines
app/shared/src/transcript/index.ts (1)
7-18: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep the golden fixtures out of the main transcript barrel.
These exports make test-only fixtures/resolvers part of the shared transcript public API. Importing
goldenTranscriptFixturesdirectly from tests/E2E keeps the production barrel smaller and avoids accidental runtime coupling to golden data.🤖 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 `@app/shared/src/transcript/index.ts` around lines 7 - 18, The shared transcript barrel currently re-exports test-only golden fixtures and resolvers from goldenTranscriptFixtures, which should not be part of the public API. Remove those exports from the main transcript index and keep the golden data accessible only via direct imports from goldenTranscriptFixtures in tests/E2E; verify any callers that rely on resolveGoldenMixedSourceTranscript, resolveGoldenMixedSourceMainTranscript, or the GOLDEN_MIXED_SOURCE_* constants are updated accordingly.
🤖 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 `@app/shared/src/transcript/index.ts`:
- Around line 7-18: The shared transcript barrel currently re-exports test-only
golden fixtures and resolvers from goldenTranscriptFixtures, which should not be
part of the public API. Remove those exports from the main transcript index and
keep the golden data accessible only via direct imports from
goldenTranscriptFixtures in tests/E2E; verify any callers that rely on
resolveGoldenMixedSourceTranscript, resolveGoldenMixedSourceMainTranscript, or
the GOLDEN_MIXED_SOURCE_* constants are updated accordingly.
In `@app/web/src/__e2e__/chat-flow-contract.spec.ts`:
- Around line 49-50: The transcript assertions in the chat-flow contract spec
are hard-coding kickoff and replay-summary text instead of deriving them from
the shared fixtures. Update the assertions in the relevant test blocks to read
the expected strings from the imported golden message/event fixtures once, and
reuse those fixture-derived values everywhere the text is checked. Use the
existing fixture imports in chat-flow-contract.spec.ts as the single source of
truth so wording changes only need to be made in one place.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: abfb3004-a52c-444b-88e4-6bca3f857fde
📒 Files selected for processing (5)
app/shared/src/transcript/goldenTranscriptFixtures.test.tsapp/shared/src/transcript/goldenTranscriptFixtures.tsapp/shared/src/transcript/index.tsapp/web/src/__e2e__/chat-flow-contract.spec.tsdocs/progress/MASTER.md
1 participant
Summary
Closes #382.
Adds a shared golden mixed-source transcript fixture for the Phase 2 chat-flow contract:
Evidence Boundary
Verification
corepack pnpm --dir app/shared exec vitest run src/transcript/goldenTranscriptFixtures.test.tscorepack pnpm --dir app/shared exec vitest run src/transcript/goldenTranscriptFixtures.test.ts src/chatview/pipeline-integration.test.ts src/transcript/normalizeHubRuntimeEvents.test.tscorepack pnpm --dir app/shared test(92 files, 794 tests)corepack pnpm --dir app/shared exec tsc --noEmit --pretty false --target ES2021 --module ESNext --moduleResolution bundler --jsx react-jsx --strict --noUncheckedIndexedAccess --exactOptionalPropertyTypes false --esModuleInterop --skipLibCheck --types vitest/globals src/transcript/goldenTranscriptFixtures.ts src/transcript/goldenTranscriptFixtures.test.tscorepack pnpm --dir app/web typecheckcorepack pnpm --dir app/web test:e2e:chat-flow(2 passed)git diff --checkpwsh ./scripts/verify/verify-doc-ssot.ps1pwsh ./scripts/verify/verify-project-skills.ps1pwsh ./scripts/verify/verify-real-e2e-contract.ps1Known unrelated broad gate:
corepack pnpm --dir app/shared lintstill fails on pre-existing Storybook/test type debt (@storybook/*,ImportMeta.env, older test typing issues). The new fixture files pass the targetedtsccommand above.Summary by CodeRabbit