fix: don't strip title when it is a name, not a schema keyword - #12219
Conversation
deepset-ai#12037 stopped `_remove_title_from_schema` from misreading the keys of a `properties` mapping as schema keywords. The same reasoning applies to five more keywords whose values are keyed by user-chosen names, and to three whose values are instance data rather than subschemas: - `$defs`, `definitions`: an entry named `title` was deleted, leaving every `$ref` that pointed at it dangling. Pydantic keys `$defs` by class name, so a nested model named `title` reaches this path through the public API and the resulting schema fails validation with `PointerToNowhere`. - `patternProperties`, `dependentSchemas`, `dependentRequired`: a rule keyed on the property name / regex `title` was silently dropped. - `default`, `const`, `enum`: a `title` key inside one of these is part of the *value*. A default of `{"title": "Untitled", "width": 80}` was emitted as `{"width": 80}`, changing the tool's contract. Keywords whose value is a genuine subschema (`items`, `propertyNames`, `additionalProperties`, `anyOf`, ...) keep losing their `title` as before; a test guards that direction too.
|
@LHMQ878 is attempting to deploy a commit to the deepset Team on Vercel. A member of the Team first needs to authorize it. |
|
Hi @LHMQ878, thanks a lot for your contribution! 🙏 We noticed that the Contributor License Agreement (CLA) check ( To get your PR reviewed, please sign the CLA via the link in the |
|
Thanks for signing the CLA, @LHMQ878! 🎉 This PR is now ready for review again and the reviewer has been re-assigned. |
ebarkhordar
left a comment
There was a problem hiding this comment.
One keyword looks like it belongs in _DATA_SCHEMA_KEYWORDS next to default/const/enum: examples. Its value is a list of instance values, Pydantic emits it straight from Field(examples=...), and the walker's list branch recurses into each example dict, so a title key that is part of the data gets deleted.
Measured on this branch (451ce7b), through the public API:
class Cfg(BaseModel):
opts: dict = Field(
default={"title": "Untitled", "width": 80},
examples=[{"title": "Untitled", "width": 80}, {"title": "Draft", "width": 40}],
)
def configure(cfg: Cfg) -> str:
"""Configure the renderer."""
tool = create_tool_from_function(configure)
tool.parameters["$defs"]["Cfg"]["properties"]["opts"]default: {'title': 'Untitled', 'width': 80} kept, as this PR intends
examples: [{'width': 80}, {'width': 40}] 'title' dropped from both
Adding "examples" to the frozenset restores it. pytest test/tools/ -m "not integration" is 292 passed both with and without the addition (python:3.12-slim, runtime deps from pyproject).
Not blocking, and whether it belongs here or in a follow-up is your call. The rest reads right to me, and the $defs case is the one that genuinely bites: a dangling $ref breaks every consumer that resolves refs, while the other four lose a validation rule quietly.
|
Good catch. Added examples to _DATA_SCHEMA_KEYWORDS in 9e7a2e7 and extended the existing instance-data regression test so a itle key inside an example is preserved while the schema-level itle is still removed. Focused ruff check and format check pass. I could not run the Hatch test command here because Hatch is not installed in this environment. |
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Coverage reportClick to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||
|
@sjrl, this seems OK to me, resuming a previous fix, and generalising to more keywords - but I think it's good if you have an extra look |
|
@ebarkhordar thanks — It's already on this branch ( |
| auto-generated ``title`` keywords no longer deletes entries of ``$defs``, ``definitions``, | ||
| ``patternProperties``, ``dependentSchemas`` or ``dependentRequired`` (which would leave a ``$ref`` | ||
| dangling or silently drop a validation rule), and no longer edits ``title`` keys inside ``default``, | ||
| ``const`` or ``enum`` values, which are instance data and part of the tool's contract. |
There was a problem hiding this comment.
Lets also mention examples here as well
There was a problem hiding this comment.
Done — added examples to the releasenote list next to default/const/enum.
|
@davidsbatista one minor comment otherwise looks good! |
|
@sjrl good call — releasenote now mentions |
|
Re-ran this against the post-merge build ( The fix is thoroughAll nine keywords the description enumerates behave correctly now, and — worth saying, since it's the half that's easy to break — the three genuine-subschema controls still lose their
It is unreleased
# haystack-ai==3.0.0, current PyPI stable
@tool
def render(cfg: Annotated[Dict[str, str], "cfg"] = {"title": "Report", "id": "x"}) -> str:
"""..."""
render.parameters["properties"]["cfg"]["default"]
# -> {"id": "x"} # pydantic emitted {"title": "Report", "id": "x"}Not a request to cut a release — just flagging that anyone who finds this thread and upgrades won't get the fix. One keyword left:
|
|
@percymcn thanks for the careful re-check — especially the OpenAPI 3.0 singular Opened a clean follow-up with the one-word fix plus a discriminating regression (unit fixture + public API): #12314 Also useful note on the fix still being unreleased on PyPI |
Follow up deepset-ai#12219: treat OpenAPI 3.0 example the same as examples so nested title keys survive tool schema stripping. Co-authored-by: Cursor <cursoragent@cursor.com>
Related Issues
No open issue — this is the remaining half of the bug class fixed by #12037. That PR stopped
_remove_title_from_schemafrom misreading the keys of apropertiesmapping as schema keywords; the same reasoning applies to eight more keywords, and nothing in #12037 or its discussion mentions them, so this looks like an unnoticed gap rather than a deferred one.Proposed Changes:
_remove_title_from_schemawalks a Pydantic-generated schema deleting everytitlekey.titleis a prose keyword, but the stringtitleis also a legal definition name, a legalpatternPropertiesregex, a legal property name, and a legal key inside adefault/const/enumvalue. The walker can't tell those apart, so it corrupts the schema in two distinct ways:1. Name-keyed maps — the key gets deleted.
$defs,definitions,patternProperties,dependentSchemas,dependentRequiredare all keyed by user-chosen names, not by schema keywords.$defsis the one that's reachable without trying: Pydantic keys$defsby class name, so a nested model namedtitleproduces a definition namedtitle, which is then deleted while the$refpointing at it survives. Through the public API:before → after this PR:
Any consumer that resolves
$refs — schema validation, an OpenAI-style strict-schema conversion, a provider-side schema check — sees a schema that points at nothing. For the other four keywords the effect is a silently dropped validation rule rather than a dangling pointer.2. Instance-data keywords — the value gets edited. The values of
default,constandenumare instance data, not subschemas. Atitlekey inside one of them belongs to the value:The model is now told the default is a dict without a title, so it can't reason about the real default and a caller relying on the schema's default writes the wrong value. No error is raised anywhere.
The fix generalises #12037's
propertiesspecial case into two module-level frozensets: name-keyed maps are recursed into by value only (keys kept verbatim), and instance-data keywords are skipped entirely. Everything else is unchanged, so keywords whose value is a genuine subschema (items,propertyNames,additionalProperties,anyOf, …) keep losing theirtitleexactly as before.Both public entry points benefit, since
ComponentTool(component_tool.py:375) calls the same helper ascreate_tool_from_function/@tool.How did you test it?
test/tools/test_from_function.py: one per affected group ($defsincl. a realjsonschema.Draft202012Validatorround-trip, the draft-07definitionsspelling, instance data, pattern/dependent keys), plus two through the public API (create_tool_from_functionwith a nested model namedtitle, and a default carrying atitlekey), plus one guarding the opposite direction —items/propertyNames/additionalPropertiesmust still lose theirtitle.haystack/tools/from_function.pyand keeping the new tests: 6 fail, 25 pass. The opposite-direction guard passes either way, as it should. With the fix: 31 passed.pytest test/tools/ -m "not integration"→ 292 passed, 12 deselected, no change in the pass set.ruff check/ruff format --checkclean;mypy haystack/tools/from_function.pyreports nothing in the touched file.releasenotes/notes/.Notes for the reviewer
properties-shaped keywords that genuinely are keyed by keywords, and being explicit keeps the failure mode a missing entry rather than a silently kepttitle.dependentRequired's values are arrays of strings, not schemas, so it only needs its keys protected; it is grouped with the name-keyed maps because theisinstance(value, dict)guard plus the value-recursion is a no-op for it.test/tools/test_from_function.pydefines a lowercaseclass title(BaseModel)at module level with# noqa: N801. The lowercase name is the point — it's what makes Pydantic emit a$defsentry namedtitle.titlereally is a keyword, including thetitlekeyword inside the$defs["title"]definition, which is still removed.Checklist