Make entities required with an empty-list default so literal models stop dropping them#2792
Closed
dimonnld wants to merge 1 commit into
Closed
Make entities required with an empty-list default so literal models stop dropping them#2792dimonnld wants to merge 1 commit into
dimonnld wants to merge 1 commit into
Conversation
…#2749) The four extraction models declared entities as optional and absent from json_schema_extra["required"]. Under a strict JSON schema a literal model resolves the optional-typed-as-objects field (while examples taught strings) by omitting it entirely — 0 entities, no error — biting the most schema-conformant models. - entities: list[Entity] = Field(default_factory=list, ...) on all four models (ExtractedFact, ExtractedFactVerbose, ExtractedFactNoCausal, VerbatimExtractedFact), with "entities" added to each required array. Empty list stays valid so lenient parsing never hard-fails; the field is no longer silently omitted. - Prompts teach the object form: concise few-shot switched to [{"text": ...}] objects, plus an explicit array-of-objects-with-text-field instruction on the base (concise/verbatim) and verbose prompts. - Deterministic schema/prompt regression tests (no LLM).
Contributor
Author
|
Closing as superseded. #2749 was already fixed in main by #2830, which took the |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2749.
Problem
The four extraction models (
ExtractedFact,ExtractedFactVerbose,ExtractedFactNoCausal,VerbatimExtractedFact) declaredentitiesas an optional field that was absent fromjson_schema_extra["required"]. Under a strict JSON schema a literal model resolves the conflict — the field is optional and typed as objects, while the few-shot examples taught bare strings — by omitting it entirely: zero entities, no error. It bites precisely the most schema-conformant models and reproduces with strict-schema both on and off.The retain parser is lenient (it coerces string entities into
Entity(text=...)), so the string-array shape itself survives — the load-bearing defect is the omitted optional field, not the string shape.Fix
entities: list[Entity] = Field(default_factory=list, ...)on all four models, and"entities"added to each model'srequiredarray. An empty list stays valid, so lenient parsing never hard-fails, but the field is no longer silently omitted by literal models.[{"text": ...}]objects, and an explicit "array of objects with atextfield, never bare strings" instruction is added to the concise/verbatim (base) prompt and the verbose prompt — so non-strict / no-schema paths stop emitting the ambiguous shape.Tests
test_fact_extraction_entities_schema.py— deterministic, no LLM:entitiesas required, typed as an array of objects with atextfield (the CI regression that would have caught this);entitiesstill parses, defaulting to[](lenient path intact);Verified locally: the schema/prompt suite is green, all prompt modes assemble, and the existing
test_fact_extraction_fact_type_prompt.pystill passes. Note the base prompt is.format()-ed twice (module assembly, then per-request), so the object examples in the few-shot are escaped accordingly and the standalone format instruction is brace-free.