← Back to PRs

#18890: fix(media): parse tool-result MEDIA directives with shared parser

by teededung open 2026-02-17 05:09 View on GitHub →
agents size: S
## Summary Fixes a false-positive media extraction path in tool results by reusing the same directive parser used for assistant output. `extractToolResultMediaPaths` previously scanned text blocks with a broad `MEDIA_TOKEN_RE`, which could capture prose text that merely contains `MEDIA:`. This change switches extraction to `splitMediaFromOutput(...)` so line/fence/path validation stays consistent. ## Changes - `src/agents/pi-embedded-subscribe.tools.ts` - Replace manual global regex scan with `splitMediaFromOutput(entry.text)`. - `src/agents/pi-embedded-subscribe.tools.media.test.ts` - Add regression test for MEDIA prose instructions that are not file paths. ## Why this matters Issue #18780 is triggered in tool-result parsing path, not only in generic media parse helpers. This keeps tool-result media extraction aligned with existing guarded parser behavior. ## Validation - `pnpm vitest run src/agents/pi-embedded-subscribe.tools.media.test.ts` Refs #18780 <!-- greptile_comment --> <h3>Greptile Summary</h3> Refactored `extractToolResultMediaPaths` to use the shared `splitMediaFromOutput` parser, fixing false-positive media extraction when tool results contain prose text mentioning "MEDIA:" without actual file paths. - **Before**: Used a manual regex scan (`MEDIA_TOKEN_RE`) with basic line-start checking that could still match documentation text - **After**: Delegates to `splitMediaFromOutput` which applies comprehensive validation (fence detection, path pattern matching, quoted string handling) - **Test coverage**: Added regression test for the prose instruction case, removed duplicate tests now covered by the shared parser's test suite in `src/media/parse.test.ts` The change consolidates parsing logic in one place, reducing maintenance burden and preventing future divergence between tool-result extraction and assistant-output extraction. <h3>Confidence Score: 5/5</h3> - Safe to merge with high confidence - well-tested refactoring that consolidates parsing logic - The PR uses an existing, well-tested shared parser (`splitMediaFromOutput`) instead of manual regex parsing. The new test case directly addresses the false-positive issue, and the removed tests are redundant since they're already covered by `src/media/parse.test.ts` (line 55-60 tests the exact "MEDIA mentions in prose" scenario). Code is cleaner and more maintainable with centralized parsing logic. - No files require special attention <sub>Last reviewed commit: 383e5d9</sub> <!-- greptile_other_comments_section --> <sub>(2/5) Greptile learns from your feedback when you react with thumbs up/down!</sub> <!-- /greptile_comment -->

Most Similar PRs