test(transcript): give the degraded-path handler test its own kb - #592
Conversation
test_handler_returns_degraded_when_absent called handle_request with no kb of its own, so _store() resolved whatever .vouch/ the cwd sat under. that is the repo's own kb on a developer checkout - the test passed locally - and nothing at all in ci, where .vouch/ is gitignored, so the handler raised KBNotFoundError and the envelope came back ok: false. the test now builds a kb under tmp_path and chdirs into it, matching the store-fixture pattern the other jsonl handler tests use, and pins VOUCH_CLAUDE_PROJECTS_DIR and CODEX_HOME at empty dirs so the absence it asserts is the raw transcript's rather than the machine's.
WalkthroughThe degraded ChangesTranscript degraded-path test
Estimated code review effort: 1 (Trivial) | ~2 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
test had already landed an equivalent hermetic fix for test_handler_returns_degraded_when_absent in c89ca20 (via #589), so both sides edited the same test and the same changelog block. resolved to test's version of the test: it uses the file's existing `store` fixture, which is the idiom the sibling transcript tests already follow (test_session_transcript.py:145 points the locator at `store.kb_dir / "none"` the same way). this branch's `, resp` assertion message survives — it is the one piece not duplicated, and it puts the envelope in the failure output when this regresses again. the changelog keeps one entry instead of two, and drops the stray `)` line c89ca20 left behind under `### Fixed`.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@tests/test_session_transcript.py`:
- Around line 214-215: Extend the assertions in the transcript-absence test to
verify that resp["result"]["reason"] starts with “raw transcript not found for
session”. Keep the existing resp["ok"] and available assertions unchanged so the
test specifically distinguishes missing transcripts from other
unavailable-transcript cases.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: cffe2065-7a70-4e66-aa03-d628085c3d67
📒 Files selected for processing (2)
CHANGELOG.mdtests/test_session_transcript.py
| assert resp["ok"] is True, resp | ||
| assert resp["result"]["available"] is False |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert the missing-transcript reason as well.
available: False also covers unreadable and oversized transcripts, so this test does not verify the intended raw-transcript-absence path. Assert that resp["result"]["reason"] starts with raw transcript not found for session.
Suggested assertion
assert resp["ok"] is True, resp
assert resp["result"]["available"] is False
+ assert resp["result"]["reason"].startswith("raw transcript not found for session")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| assert resp["ok"] is True, resp | |
| assert resp["result"]["available"] is False | |
| assert resp["ok"] is True, resp | |
| assert resp["result"]["available"] is False | |
| assert resp["result"]["reason"].startswith("raw transcript not found for session") |
🤖 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 `@tests/test_session_transcript.py` around lines 214 - 215, Extend the
assertions in the transcript-absence test to verify that
resp["result"]["reason"] starts with “raw transcript not found for session”.
Keep the existing resp["ok"] and available assertions unchanged so the test
specifically distinguishes missing transcripts from other unavailable-transcript
cases.
What changed
test_handler_returns_degraded_when_absentnow builds its own KB undertmp_pathand chdirs into it before callinghandle_request, matchingthe
storefixture pattern the other jsonl handler tests already use. italso pins
VOUCH_CLAUDE_PROJECTS_DIRandCODEX_HOMEat empty dirs, sothe absence it asserts is the raw transcript's rather than whatever the
machine happens to have.
Why
ciis red ontestitself (be43b22), and every PR based on it inheritsthe failure — that's what #589 is hitting.
the test called
handle_requestwith no KB of its own, so_store()resolved whatever
.vouch/the cwd sat under. on a developer checkoutthat's the repo's own KB, so it passed locally.
/.vouch/is gitignored,so in CI there is nothing to find:
discover_rootraisedKBNotFoundError,_h_session_transcriptturned it into aRuntimeErrorbefore
load_transcriptever ran, and the envelope came backok: false—assert False is True.the two sibling handler tests were never affected because they fail
validation before
_store()is reached.What might break
nothing. test-only change; no
.vouch/layout, kb.* behaviour oron-disk shape is touched.
VEP
not a surface change.
Tests
make checkpasses locally (lint + mypy + pytest).vouch-less worktree, which is the actual CIcondition:
1907 passed, 30 skipped, exit 0. the same worktree onthe parent commit reproduces
1 failed.CHANGELOG.mdupdated under## [Unreleased]Summary by CodeRabbit