Repository navigation
feat(voice-agent): in-session artifact cache for fast document Q&A (closes #642) - #828
Conversation
…loses sonichi#642) Adds set_active_artifact / query_active_artifact / clear_active_artifact inline tools so voice can review a document without task-bridge round-trips on every turn. Reduces 5-turn paper-review latency from ~6 min to ~2.5s after initial load. - src/artifact-cache-tools.ts: new module with module-level cache, pdftotext extraction for .pdf, section splitting for .md/.txt and .ts/.js/.py/.sh, keyword-scan fallback for no-section-match queries - inline-tools.ts: re-export + register the three new tools in inlineTools and ownerOnlyTools; export clearActiveArtifact() for voice-agent wiring - voice-agent.ts: call clearActiveArtifact() on onSessionEnd to prevent cross-session state leaks - tests/active-artifact.test.ts: 11 tests covering set/query/clear lifecycle, section matching, keyword scan, no-match, auto-clear, and artifact replacement
qingyun-wu
left a comment
There was a problem hiding this comment.
Cold review — LGTM in shape. The 80s → 300ms latency claim is a meaningful win for the document-Q&A use case.
Verified from the diff
- `activeArtifact` is module-scoped singleton (one cached doc per voice-agent process) — matches "in-session" framing
- `clearActiveArtifact()` called from voice-agent's `onSessionEnd` — prevents cross-session state leaks (the right invariant)
- Path expansion handles both `~` and `${ENV}` — appropriate for voice-spoken file paths
- 11 new tests, suite passes (per body)
Non-blocking observations
-
Path expansion regex is slightly aggressive: `\${([A-Z_]...)\}|\$([A-Z_]...)` matches uppercase-only env vars. Lowercase ones (rare but legal) won't expand. If a user says `set active artifact ~/Documents/$ws/paper.pdf` and their env has a lowercase `ws`, expansion silently no-ops. Worth a one-line "lowercase env vars not supported" comment, or relax to `[A-Za-z_]`.
-
PDF fallback to "raw read" if pdftotext unavailable — for a binary PDF without pdftotext, reading raw bytes will produce garbage in the query engine. Worth a probe at `set_active_artifact` time that returns an explicit "pdftotext required for .pdf" error instead of silent garbage. The user will get unhelpful answers if the fallback kicks in.
-
Section splitter for code files uses regex on `class`, `function`, `const X =` — works for top-level declarations but will miss `def` in Python, methods inside classes, and arrow functions assigned later. For v1 this is probably enough; flag a followup for "code-aware splitter" if voice Q&A on code becomes a hot path.
-
Cross-session leak via process-level singleton. Single-process voice agent is fine. If the agent ever forks workers or shares the cache across sessions, the singleton becomes a leak — but `onSessionEnd → clearActiveArtifact` already guards. Just worth a code comment that the lifetime contract is "one voice session at a time."
-
Auto-clear edge case. What if the user explicitly calls `set_active_artifact` mid-session for a different file? Looks like the new one overwrites the old (`activeArtifact = ...`) — that's the right behavior, but worth confirming `clear → set` is also exercised by a test.
-
No upper-bound on artifact size. A 100 MB PDF could blow the process's heap. Consider a `MAX_NCHARS = 5_000_000` (~5 MB text) guard in `set_active_artifact` that returns a "too large" error rather than OOM-ing the agent.
Closes #642. The numbers in the body (80s → 300ms) are the right kind of metric to ship with — keep doing that.
Ship after the path/PDF-fallback nits, or merge and follow-up.
…loses sonichi#642) (sonichi#828) Adds set_active_artifact / query_active_artifact / clear_active_artifact inline tools so voice can review a document without task-bridge round-trips on every turn. Reduces 5-turn paper-review latency from ~6 min to ~2.5s after initial load. - src/artifact-cache-tools.ts: new module with module-level cache, pdftotext extraction for .pdf, section splitting for .md/.txt and .ts/.js/.py/.sh, keyword-scan fallback for no-section-match queries - inline-tools.ts: re-export + register the three new tools in inlineTools and ownerOnlyTools; export clearActiveArtifact() for voice-agent wiring - voice-agent.ts: call clearActiveArtifact() on onSessionEnd to prevent cross-session state leaks - tests/active-artifact.test.ts: 11 tests covering set/query/clear lifecycle, section matching, keyword scan, no-match, auto-clear, and artifact replacement Co-authored-by: Qingyun Wu <qingyun0327@gmail.com>
Summary
set_active_artifactis answered from in-process memory, with no task-bridge round-trip.set_active_artifact,query_active_artifact,clear_active_artifactChanges
src/artifact-cache-tools.tssrc/inline-tools.tsinlineToolsandownerOnlyToolssrc/voice-agent.tsclearActiveArtifact()inonSessionEndtests/active-artifact.test.tsFile-type support
.pdfpdftotext -layout(falls back to raw read if unavailable).md,.txt^#—^##—^###headers.ts,.js,.py,.shclass,function,const X =top-level declarationsTest plan
npm test— 173 tests, 0 failures (11 new tests for this feature)npx tsc --noEmit— cleanset_active_artifact("path/to/paper.pdf")→query_active_artifact("contribution")→ verify <500ms responseCloses #642
🤖 Generated with Claude Code