fix(curate): improve Vertex AI reliability for large curation extractions#79
fix(curate): improve Vertex AI reliability for large curation extractions#79mpeter wants to merge 2 commits into
Conversation
yvonnedevlinrh
left a comment
There was a problem hiding this comment.
Thanks for the contribution, @mpeter! The root cause analysis in all three issues were great, made the review straightforward.
Build, vet, and all tests pass locally (go test -race -count=1 ./...). The runtime logic is correct, I verified the fallback path handles string, integer, and boolean quality_flags values.
Blocking (1 item):
- Missing regression test for the quality_flags string fallback (bug #78). This is the most significant logic change in the PR and the only path without test coverage. A TestParseExtractionResponse_QualityFlagsString that passes "quality_flags": "none" and verifies the item is parsed with empty quality flags would cover it.
Non-blocking (2 nits, 1 observation):
- See inline comments on curate/curate.go
- The prompt simplification reduces quality metadata granularity (removing detailed quality_flags type guidance). Reasonable trade-off for token reduction, but worth noting as a deliberate behavior change beyond the three bug fixes.
| Tag string `json:"tag"` | ||
| Category string `json:"category"` | ||
| Confidence string `json:"confidence"` | ||
| QualityFlags json.RawMessage `json:"quality_flags_raw"` |
There was a problem hiding this comment.
nit: This field is captured but never read - the constructed KnowledgeFile at line 364 doesn't use tmp.QualityFlags. You can remove it from the struct; json.Unmarshal will silently ignore the unknown key.
| var f KnowledgeFile | ||
| if err := json.Unmarshal(raw, &f); err != nil { | ||
| // quality_flags may be a string — normalise and retry. | ||
| normalised := strings.ReplaceAll(string(raw), `"quality_flags":`, `"quality_flags_raw":`) |
There was a problem hiding this comment.
nit: The string-level rename works correctly (I verified the edge cases), but for a future cleanup consider unmarshalling into map[string]json.RawMessage instead. That avoids string manipulation on raw JSON entirely and handles any unexpected type for any field.
|
Addressed review feedback: added the missing quality_flags-string regression test, and replaced the string-manipulation fallback with a map[string]json.RawMessage approach (also removes the unused struct field). |
…ions Three bugs causing curation failures on real-world vaults with moderate content density (9-42 documents per store): Bug 1 — VertexSynthesizer MaxTokens hardcoded at 4096 (unbound-force#76) Real curation extractions produce 4K-16K output tokens. Claude truncates mid-JSON at the 4096 limit → ParseExtractionResponse fails with 'unexpected end of JSON input'. Raised to 16000. llm/vertex.go: MaxTokens 4096 → 16000 Bug 2 — VertexSynthesizer HTTP timeout too short for large prompts (unbound-force#77) Vertex AI with Claude Sonnet takes 120-260s for 30K-40K token prompts. The 120s HTTP client timeout consistently cuts off large extractions. Raised to 300s (Vertex has different latency than local Ollama). llm/vertex.go: Timeout 120s → 300s Bug 3 — ParseExtractionResponse strict unmarshal fails on quality_flags string (unbound-force#78) LLMs non-deterministically output 'quality_flags': 'none' (string) instead of 'quality_flags': [] (array). Strict json.Unmarshal into []KnowledgeFile rejects the entire response. Changed to parse []json.RawMessage first, then unmarshal each item individually with a permissive fallback that accepts any type for quality_flags and discards non-array values. curate/curate.go: strict unmarshal → per-item with fallback Also improved the extraction prompt to request concise output (3-8 items per document, short excerpts, terse content) which reduces token consumption and makes truncation less likely even if the max_tokens limit is hit. Tested against 3 real accounts (42, 10, 9 documents) on Vertex AI with claude-sonnet-4-6@default. All stores now curate successfully.
Add the missing regression test for the quality_flags string fallback (bug unbound-force#78), and replace the string-manipulation JSON fallback with a map[string]json.RawMessage approach that drops the unparseable field directly instead of renaming it — this also removes the unused QualityFlags field from the intermediate struct.
068b2a6 to
e0579b5
Compare
Fixes three bugs that cause
dewey curateto fail consistently on real-world vaults with moderate content density.Bugs Fixed
#76 — MaxTokens hardcoded at 4096 in VertexSynthesizer
Real curation extractions produce 4K-16K output tokens. Claude truncates the JSON array at the 4096 token limit, causing
ParseExtractionResponseto fail withunexpected end of JSON input.#77 — HTTP timeout too short for Vertex AI
Vertex AI with Claude Sonnet takes 120-260s for 30K-40K token prompts. The 120s HTTP client timeout cuts off large extractions mid-stream. Vertex has fundamentally different latency characteristics than local Ollama inference.
#78 — quality_flags string type crashes ParseExtractionResponse
LLMs non-deterministically output
"quality_flags": "none"(string) instead of"quality_flags": [](array). Strictjson.Unmarshalinto[]KnowledgeFilerejects the entire response for a type mismatch on a single field.Changed to parse
[]json.RawMessagefirst, then unmarshal each item individually. Items that fail strict unmarshal fall back to a permissive struct that accepts any JSON type forquality_flagsand discards non-array values. Valid items are preserved; unparseable items are logged and skipped.Additional: prompt conciseness
The extraction prompt now requests concise output (3-8 items per document maximum, short excerpts ≤20 words, content ≤30 words per item). This reduces output token consumption, making truncation less likely even near the
max_tokenslimit, and produces more scannable curated files.Testing
Tested against 3 knowledge stores with varying content density (9, 10, and 42 documents respectively). All previously failed with one or more of the three bugs above and now produce knowledge files successfully.
Closes #76, #77, #78