Repository navigation
FIX: query and remove seeds by literal metadata keys - #3125
Roman Lutz (romanlutz) merged 5 commits into
Conversation
`prompt_metadata` is documented as a free-form dictionary, but the SQLite
and Azure SQL backends spelled each key into the JSON path and reused it
as the bind-parameter name:
JSON_EXTRACT(prompt_metadata, '$.{key}') = :{key}
Only `[A-Za-z0-9_]` survives that. Anything else raises before the query
runs, so the filter cannot be expressed at all:
ArgumentError: This text() construct doesn't define a bound
parameter named 'attack.id'
`get_message_pieces_async(prompt_metadata={"attack.id": "v"})` against
SQLiteMemory raises that today. A dot was also read as a path step, so a
key that happened to survive would have looked up a nested object.
Both backends now bind the path with the key quoted into it (`'$."{key}"'`)
and give each condition positional parameter names, matching how microsoft#3063
fixed the same interpolation in the label conditions:
JSON_EXTRACT(prompt_metadata, :pm_path_0) = :pm_value_0
Binding the path also keeps the value out of the statement text entirely.
Seed metadata filtering goes through the same two code paths
(`_get_seed_metadata_conditions`), so it is fixed as well.
Regression tests query four keys holding a dot, hyphen, space and colon,
plus one query naming three keys at once. Against the parent commit they
fail with the ArgumentError above; the plain-key cases the suite already
covered still pass.
Not addressed: a key containing a double quote cannot be expressed in a
SQLite or T-SQL JSON path, so it matches nothing rather than raising.
That is the same residual microsoft#3063 left in the label path.
Validation at this head: tests/unit/memory/memory_interface/,
tests/unit/memory/test_sqlite_memory.py, tests/unit/memory/test_azure_sql_memory.py
-- 648 passed, 1 skipped; ruff check and format clean.
Chen Yufeiyang (feiiiiii5)
changed the title
|
Thanks for reproducing the deletion case. Fixed in Added quote and backslash cases for prompt/seed retrieval, seed removal and group removal, each asserting that unrelated records remain. All eight fail on The PR description now reflects the literal-key implementation and removes the incorrect claim that quoted keys would simply match nothing. |
|
Thanks Roman Lutz (@romanlutz) — the shared Azure predicate was missing that length guard. Fixed in Added generated-MSSQL predicate checks for trailing spaces, empty versus space-only keys, Unicode and multiple filters; all three fail on The memory suites pass: 683 passed, 1 skipped. Ruff check/format passed. Live SQL Server was not run; the Azure regression checks inspect generated SQL, and the PR description now makes that limit explicit. |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Keep the production literal-key fix already merged in microsoft#3125 and retain additional saved/reopened retrieval, deletion, value-type and MSSQL binding coverage for microsoft#3180. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1696f5bb-ccc6-41e1-a983-7cceb2ec6e3c
2 participants
Description
Prompt and seed metadata accept free-form keys, but keys with dots, spaces or punctuation were used as bind-parameter names and JSON paths. Quoting paths is insufficient: quotes or backslashes inside a key can select a different record, including when deleting seeds.
Changes
SQLite compares literal keys through
json_each(...).key; Azure SQL comparesOPENJSONkeys. Azure also checks Unicode byte lengths, since SQL Server equality pads trailing spaces. Keys and values remain bound parameters; value comparison semantics are unchanged. Prompt retrieval, seed retrieval and seed/group removal share these filters. Related: #3063, which addressed label filters.Tests
Eight quote/backslash retrieval and removal regressions fail on
fab86809with unrelated records selected or removed. Three generated-MSSQL predicate tests fail on2efea640because exact-key length guards are absent. These inspect generated SQL, rather than executing SQL Server.SQLite database tests distinguish
plainfromplain, empty from space-only keys, and Unicode keys with trailing spaces, including removal. Those cases already pass on2efea640and pin the behavior Azure should preserve.Tests and Documentation
Memory-interface, SQLite and Azure unit suites: 683 passed, 1 skipped. Ruff check and format check passed on the three newly changed files. No docs changed; JupyText was not run.
Not addressed
Live Azure SQL integration was not run. Azure checks cover generated MSSQL predicates and literal bound keys; SQLite checks execute against a local database.