fix(schema): emit unique_identifiers for @upsert/@lookup nodes#48
Open
shawkatkabbara wants to merge 2 commits into
Open
fix(schema): emit unique_identifiers for @upsert/@lookup nodes#48shawkatkabbara wants to merge 2 commits into
shawkatkabbara wants to merge 2 commits into
Conversation
deeptrust cookbook
build_schema_params() was producing node_types without the unique_identifiers field, even though SchemaCreateParams documents it as the merge key for @upsert/@lookup nodes (equivalent to required + search=exact() properties). Without unique_identifiers, the memory server's _get_unique_identifiers_for_node_type() returns [] and falls back to _create_node_with_content_check, creating duplicate nodes for the same logical entity (e.g. multiple :call nodes sharing the same call_id). This patch derives unique_identifiers from properties marked required=True + search=exact() and emits them on @upsert and @lookup nodes. Repro: register a schema with @upsert + prop(required=True, search=exact()) and inspect the resulting node_type — the field was empty before, populated now. Tested locally: schema build now produces unique_identifiers=['call_id'] for an @upsert call type and ['id'] for a @lookup tactic type.
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.
Problem
build_schema_params()was producing node_types without theunique_identifiersfield, even thoughSchemaCreateParamsdocuments it as:Without this field, the memory server's
_get_unique_identifiers_for_node_type()returns[]and falls back to_create_node_with_content_check, creating duplicate nodes for the same logical entity.Reproduction
Register a schema with
@upsert+prop(required=True, search=exact()):Before this PR:
{ "name": "call", "properties": {...}, "unique_identifiers": [] // ❌ empty }After this PR:
{ "name": "call", "properties": {...}, "unique_identifiers": ["call_id"] // ✅ populated }Real-world impact
In production data we observed 11 duplicate
:callnodes sharing identicalcall_idvalues, caused entirely by this gap. After running with the fix, only one node is created percall_id(server-side MERGE onunique_identifierssucceeds).Fix
Derive
unique_identifiersfrom properties markedrequired=True+search=exact()(matching the documented contract) and emit on@upsertand@lookupnodes.Tests
Locally verified:
@upsert callwithcall_id: str = prop(required=True, search=exact())→unique_identifiers: ["call_id"]✓@lookup tacticwithid: str = prop(required=True, search=exact())→unique_identifiers: ["id"]✓@create(default) policy → field omitted (only emitted on upsert/lookup) ✓