feat: make Tavily web search query context-aware when PDF is uploaded#258
feat: make Tavily web search query context-aware when PDF is uploaded#258nkmohit wants to merge 3 commits intoTHU-MAIC:mainfrom
Conversation
cosarah
left a comment
There was a problem hiding this comment.
PR #258 Review: Context-Aware Web Search Query Rewrite
What Was Done Well
- Clean extraction of a shared
buildSearchQueryhelper — eliminates duplication between/api/web-searchandgenerateClassroom. - Solid best-effort / graceful-degradation pattern: every failure path falls back to the raw query with logging.
SearchQueryBuildResultmetadata is well-designed for observability.- Prompt templates follow existing project conventions.
Issues
Important
1. maxOutputTokens set to model's full outputWindow for a tiny query-rewrite task
app/api/web-search/route.ts:56
maxOutputTokens: modelInfo?.outputWindow,The rewrite prompt asks for a single JSON object with a query string under 320 chars. The model's outputWindow can be 128k tokens. Some providers allocate resources or charge based on the requested maxOutputTokens. Consider capping to a small value (e.g., 256–512 tokens) to avoid waste and speed up responses. The same pattern in classroom-generation.ts:200 is reasonable since it generates full scenes, but for a query rewrite it's excessive.
2. Double-normalization in shouldRewriteSearchQuery
lib/server/search-query-builder.ts:34-36
shouldRewriteSearchQuery normalizes its inputs internally, but buildSearchQuery on line 47 passes already-normalized values into it. Not a bug (result is the same), but confusing and wasteful. The function should either accept raw inputs or skip re-normalization.
3. PDF text accepted without size limits at the API boundary
app/api/web-search/route.ts:29
pdfText is accepted with no validation or truncation at the API layer. normalizePdfExcerpt truncates to 7000 chars before the LLM, but a client could still send an arbitrarily large payload that gets parsed and held in memory. Consider adding an explicit check/truncation at the API boundary, or at minimum documenting the reliance on the framework body limit.
Minor
4. Content-Type header set twice
app/generation-preview/page.tsx:312-314
getApiHeaders() already sets 'Content-Type': 'application/json'. The explicit override on line 314 is redundant — remove it.
5. User prompt template contradicts itself on code fences
lib/generation/prompts/templates/web-search-query-rewrite/user.md:13-18
Says "no code fences" then immediately shows a code-fenced JSON example. This could confuse the LLM. Show the example without fences, or rephrase the instruction.
6. Inconsistent callLLM calling convention
app/api/web-search/route.ts:51-54 uses { system, prompt } while classroom-generation.ts:196-199 uses { messages: [...] } for the same logical operation. Minor consistency gap.
Summary
Well-structured PR with clean architecture and robust fallback behavior. The most actionable item is Issue #1 — capping maxOutputTokens for the rewrite call would be a meaningful cost/latency improvement for what is effectively a one-line JSON generation. The other issues are minor consistency and hygiene items.
Verdict: request changes (for issue #1 primarily)
|
Thanks for the detailed review, @cosarah. I pushed a follow-up commit addressing the review items. Changes made:
The rewrite behavior itself is unchanged: it is still best-effort and still falls back to the normalized raw requirement if the rewrite step is unavailable or unusable. One small note on the pdfText clamp: it now limits what the rewrite flow sees, but it still happens after req.json(), so it does not change request-parse memory behavior. Ready for re-review when you have time. |
Summary
This PR makes Tavily web search queries context-aware when a PDF is attached, and aligns both active generation paths with the same rewrite behavior.
Previously, the preview flow sent the raw requirement directly to
/api/web-search, so vague prompts like “Tell me about this paper” were passed to Tavily unchanged even when parsed PDF text was available. This PR adds a shared server-side query rewrite step that can use uploaded PDF text and also improves overly long raw queries before they reach Tavily.Related Issues
Fixes #246
Changes
web-search-query-rewrite/api/web-searchto:pdfTextgeneration-previewto send parsedpdfTextto/api/web-searchgenerateClassroom(...)to reuse the same shared query-rewrite helper instead of keeping separate local rewrite logicType of Change
Verification
Steps to reproduce / test
pnpm devgeneration-previewflow with:/api/web-searchis called withpdfTextand that the request succeeds instead of sending only the raw vague requirement/api/generate-classroomdirectly withpdfContent.textandenableWebSearch: trueto exercise the server classroom-generation path as wellWhat you personally verified
pdfTextinto/api/web-search/api/web-searchuses the same header-based model resolution pattern as the other preview generation APIshasPdfContext: truerewriteAttempted: true/api/web-searchandgenerateClassroom(...)generateClassroom(...)base model selection, where that job path still uses server-default model resolution and may fail if the server default provider is not configuredEvidence
pnpm check && pnpm lint && npx tsc --noEmit)PDF attached: web search rewrite uses PDF context and returns relevant sources.

No PDF: long requirement is rewritten before Tavily and still returns relevant sources.

Checklist