Skip to content

kividb: exact TAG filter values on 1.1.2 (escaping + live fixture), follow-up to #381 - #375

Merged
fcostaoliveira merged 21 commits into
masterfrom
kividb-1.0.5-multivalued
Oct 29, 2026
Merged

fcostaoliveira merged 21 commits into
masterfrom
kividb-1.0.5-multivalued

Conversation

@fcostaoliveira

@fcostaoliveira fcostaoliveira commented Oct 29, 2026 •

Copy link
Copy Markdown
Collaborator

Follow-up to #381 (merged as 5fe1278, by @murtazayusuf). It started as #326, also by @murtazayusuf, whose commit ce46925 is the first commit on this branch.

Thank you, Murtaza. The version gate, the kividb_caps module, SEPARATOR ; on 1.1.2 and the 1.0.5 and 1.1.2 re-measurements are yours and are already on master. This PR adds what #381 left open: every TAG filter value is exact on 1.1.2, or a hard error.

The problem on master

#381 sends TAG filter values raw. Measured on kividb/kividb:v1.1.2-full (throwaway CI probe #382), 22 of 146 probe values come back wrong when sent raw:

  • a\b matches ab;
  • J\"org Hennig matches J"org Hennig;
  • x|y matches x and y;
  • a } or a leading " is a syntax error.

arxiv-filters is the shipped dataset this affects. 77 of its 10,000 queries contain \, 14 contain braces and 4 contain parentheses. Raw, these return 0 documents or fail per query. search() drops a failed query with only a log line, so recall comes out silently wrong.

What this changes (6 files against master)

Query side (src/kividb_caps.rs, library).

  • tag_literal escapes every TAG value: a backslash before each ASCII punctuation character and space, which is the RediSearch rule.
  • On 1.1.2 the escaped form is exact for all 32 ASCII punctuation characters plus space, at the start, middle and end of a value, bare, grouped and AND'd. That includes ( ) | @, so parentheses are not refused.
  • tag_value_problem turns values with no exact encoding into hard errors:
    • ; (the separator);
    • a trailing * (a prefix wildcard even when escaped);
    • a leading $ (a query parameter even when escaped);
    • leading or trailing whitespace (trimmed).
  • Two more are hard errors because they are unmeasured:
    • an empty value;
    • whitespace other than a space or a control character inside a value (tab, newline, CR, VT, FF, U+00A0, U+3000, …).
  • A scan of every shipped TAG query value (h-and-m, arxiv, the three random-keyword sets, tenants-100) finds none of these shapes.
  • On a server where TAG fields are unsupported (older than 1.1.2, or no readable version), every TAG value is a hard error. This also covers a string condition on a field the schema doesn't declare.

Engine (kividb.rs, filter_guard.rs).

  • tag_clause and every match_any arm go through tag_literal.
  • An empty value is no longer silently turned into a never-match.
  • The raw-value refusal path (check_tag_value / tag_value_offender) and its tests are replaced.

Live fixtures (tests/integration_kividb.rs, tests/common/mod.rs, KiviDB 1.1.2).

  • Backslash / brace / quote values and ( ) | @ values, each next to its decoy, run with --fail-on-dropped-queries.

  • test_binary_kividb_tag_values_exact_counts. Each value is stored on its own number of documents. The engine runs the dataset (--fail-on-dropped-queries --keep-data, recall >= 0.95). Each value is then queried on the engine-built index with the engine's own tag_literal, bare and as (A | none) A, and must return exactly its expected document ids:

    value docs value docs
    J\"org Hennig 12 Smith, John 26
    J"org Hennig 14 Smith 28
    Rasmus Bj{\o}rk 16 Francisco (Paco) Guinea 30
    Rasmus Bjork 18 Francisco 32
    a\b 20 f(x) 34
    ab 22 x|y 36
    close}brace 24 x 38

README. The KiviDB note covers the escaping rule and its 1.1.2 measurements, and the full hard-error list including the two unmeasured shapes. It also states that refusing the 11 scalar-TAG datasets below 1.1.2 narrows support: they ran correctly on the old 1.0.2 pin. The arxiv count check is labelled correctly: 2,123,402 submitter values across 2,138,591 documents, checked on 1.0.5.

Verification

  • make agent-check: exit 0 (464 lib + 969 bin tests, plus the invariant suites). The head's tree is identical to the gated commit.
  • Live mutation, throwaway draft tmp: #375 mutation check, TAG escaping reverted (throwaway, do not merge) #383 (closed, branch deleted): with escaping reverted, the KiviDB job went red, 3 of 26 failing. The exact-count and backslash/brace runs failed on brace syntax errors, and the paren fixture scored recall 0.630.
  • Unit mutations, each restored afterwards, all red:
    • escaping reverted;
    • \ not escaped;
    • tag_literal ignoring server support;
    • each of the four hard errors allowed;
    • empty value allowed;
    • the interior-whitespace check removed;
    • that check narrowed to is_ascii_whitespace (which misses U+00A0, U+3000 and U+2028);
    • the control-character check removed;
    • a space refused too (positive control).

History and merging

  • Squash-merge, please, and trim the squash text. The branch carries commit messages that later commits reversed: the 1.0.5 gate, "keep pre-1.0.5 KiviDB live-tested" and the legacy job. Keep the Co-authored-by trailer below.

  • b7dda91 differs from 62ada3a. It is @murtazayusuf's 62ada3a cherry-picked with his author and message intact, but resolved against this branch's escaping work:

    • TagSemantics became tags_supported: bool;
    • the 1.0.2 raw-emission branch and its tests were dropped;
    • the round-4 refusal details were kept;
    • a prose paragraph listing the measured versions was added to TAG_MIN_VERSION's doc.

    It carries no cherry-pick trailer, so git blame on b7dda91 shows him for code he didn't write. The squash avoids that.

  • origin/master (with 5fe1278) is merged in as a merge commit. The merge needed no hand edits beyond keeping this branch's side, and git diff origin/master...HEAD is this follow-up only.

Deferred

Co-authored-by: murtazayusuf 65263321+murtazayusuf@users.noreply.github.com

🤖 Generated with Claude Code

claude and others added 8 commits October 4, 2026 11:39
….0.5-full

KiviDB 1.0.5 accepts FT.CREATE `TAG SEPARATOR` and splits stored values on
it, so the multi-valued `labels` field no longer needs to be a hard error.

- New `kividb_caps` library module (unit-tested): parses `kividb_version`
  from `INFO server` and gates multi-valued TAG support at >= 1.0.5. An
  unknown, malformed or pre-release version reads as unsupported, so the
  existing hard error remains the fallback.
- `configure()` and the `--skip-upload` path of `search()` read the
  server version before `validate_dataset_support`; on >= 1.0.5 `labels`
  is declared `TAG SEPARATOR ;`, the same declaration redis.rs uses and
  matching the `;`-joined value `upload` already stores. `create_index`
  refuses to declare it without support even if validation is skipped.
- The labels integration test now asserts filtered recall (>= 0.9) on the
  multi-valued fixture instead of rejection; the rejection path is covered
  by unit tests on both sides of the gate.
- tests/docker-compose.test.yml: quay.io/kividbio/kividb:v1.0.2-full ->
  v1.0.5-full.
- README / module docs: the #205 divergences re-measured on 1.0.5. Items
  1-7 and 9 no longer reproduce; 8 (no GEO type) still does. The emitter
  is unchanged because its output is correct on every version.

Verification:
- make agent-check: exit 0 (181 lib + 906 bin + invariant suites, fmt,
  clippy -D warnings).
- integration_kividb against KiviDB 1.0.5: 21/21 passed;
  multi-valued labels recall 1.000.
- Mutations, each restored after: dropping `SEPARATOR` from the labels
  declaration -> labels test recall 0.000, FAILED; gate min version
  1.0.4 or always-true -> kividb_caps unit tests FAILED; rejecting labels
  regardless of version -> engine unit test FAILED.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WdtvAfcNSzV85y5jAHMVr8
Brings #326 (ce46925) up to master 50b38e1 (#341 hybrid, #342 p90 +
streaming upload, #343/#346 numeric range, #344 layouts + workload
switch, #357/#360/#361 incl. KiviDB FLAT readiness, #369 build
provenance).

One textual conflict, src/lib.rs: keep master's new modules and add
`kividb_caps` in alphabetical order. tests/docker-compose.test.yml
auto-merged to #326's v1.0.5-full pin (master still pinned v1.0.2-full;
CI and the Makefile both start the compose service, so that is the only
pin).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
KiviDB 1.0.5 did not only start accepting `SEPARATOR`: a bare TAG now
splits on RediSearch's default `,`. Measured: with `s TAG`, a stored
`Smith, John` makes `@s:{Smith}` match it and `@s:{Smith, John}` match
nothing (Redis 8.8.0 answers identically, which is why redis.rs declares
every TAG `SEPARATOR ;`). #326 kept scalar keyword/uuid/bool bare, so on
1.0.5 a filter value containing a comma silently under-matches.

Probed every published image around the boundary (bare TAG vs
`SEPARATOR ;`, values `Smith, John` / `a;b;c`):
  1.0.2, 1.0.3, 1.0.4-rc3, 1.0.4: SEPARATOR rejected, bare TAG atomic
  1.0.5-rc1, 1.0.5, 1.0.6-rc4:    SEPARATOR splits, bare TAG splits on `,`

- kividb_caps: `KividbServer::from_info` -> `tag_semantics()`
  (Atomic / Separator / unknown) and `describe()`. A pre-release is gated
  on its core version: 1.0.5-rc1 already has 1.0.5's semantics, so the
  #326 rule (pre-release = unsupported) put it on bare TAGs, which it
  splits on `,`. Parser takes SemVer shape (`-pre`, `+build`), rejects
  `v1.0.5`, `+1.0.5` etc. (u32::from_str accepts a leading `+`).
- kividb.rs: `tag_declaration` (pure, unit-tested) gives every TAG field
  `SEPARATOR ;` on Separator servers, bare on Atomic, and refuses labels
  on Atomic and any TAG on an unknown version; `create_index` uses it,
  so it is also the backstop if validation is skipped.
- `validate_dataset_support` takes the server identity: unknown version
  + any TAG field is refused (neither declaration is safe), labels on
  Atomic refused as before. Every message names what `INFO server`
  reported (#326 review, optional item 1).
- Geo is checked by `validate_schema_static` before `configure()`
  connects (#326 review, optional item 4).
- `search()`: probe, validation and index-existence guard share one
  connection, still before `read_queries()` and the worker gate.
- Integration: new `test_binary_kividb_keyword_value_with_comma`
  (`Smith, John` / `Smith` / `John`, queries alternate the first two, so
  both under- and over-match fail). The labels test asserts the server
  has the semantics `KIVIDB_EXPECT_TAG_SEMANTICS` declares (default
  `separator`) and then asserts the filtered path or the configure-time
  rejection, so it stays meaningful on a pre-1.0.5 server.

Verified: integration_kividb 22/22 on v1.0.5-full, and 22/22 on
v1.0.2-full with KIVIDB_EXPECT_TAG_SEMANTICS=atomic.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
#326 moved the only KiviDB CI service to v1.0.5-full, leaving the
pre-1.0.5 branch (bare TAGs, labels rejected) covered by unit tests only
(#326 review, optional item 2).

- tests/docker-compose.test.yml: `kividb-legacy` service on
  v1.0.2-full (port 6387), the image CI ran until now.
- Makefile: `integration-test-kividb-legacy` runs integration_kividb
  against it with KIVIDB_EXPECT_TAG_SEMANTICS=atomic.
- ci.yml: the KiviDB job runs the same binary against it after the
  1.0.5 run (the suite takes ~3 s per server).
- README KiviDB note: row 10 (bare TAG splits on `,` from 1.0.5) and the
  measured version boundary; unknown-version refusal; `;` inside a value
  is a separator, as on Redis (review item 5); geo rejected before
  connecting; "61" -> "62" datasets; indexing-wait sentence updated for
  #361's FLAT rule.
(The matching kividb.rs doc notes, i.e. what changed on the index side
and the stored-`|`/`@` inertness re-measured under `SEPARATOR ;`, are
in the previous commit.)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…hable

Mutation check: deleting the "digits only" check left every test green,
because `split_once('+')` (build metadata) already cuts `+1.0.5` and
`1.+0.5` before any component reaches `u32::from_str`. That split and
the pre-release validity check are the real guards; both go red when
mutated (rejects_malformed_versions, parses_prerelease_and_build_metadata).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
….rs from 1.0.5

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 29, 2026 •

Copy link
Copy Markdown

🤖 Automated first-pass review — a human maintainer's review is still required before merge.

What I checked: the encoding path from parse_conditions → build_filter → tag_clause / build_match_any → kividb_caps::tag_literal; the tag_value_problem refusal set and the order of its checks; the gate that threads tags_supported from search()'s one INFO server read into the prefilters; the filter_guard.rs matrix arm; the three new live fixtures in tests/common/mod.rs and tests/integration_kividb.rs; and the README rewrite against the code.

Against this repo's main bug class (taxonomy item 1, "still runs, still reports a number, the number is wrong"), this PR fixes a real silent wrong-answer on master and adds none of its own:

  • On master, a value that can't be encoded is sent raw: per the PR body's 1.1.2 probe, 22 of 146 probe values come back wrong, and on arxiv-filters 77 of 10,000 queries return 0 docs or a dropped query. After this PR every such value goes through tag_literal.
  • tag_literal either returns the escaped literal or an Err. The Err propagates out of try_resolve_all before read_queries(), so nothing degrades to a never-match or an unfiltered run.
  • The empty-value path that used to turn into never_match_tag (old tag_clause, and the continue in build_match_any) is now a hard error too. That is the right direction, since whether KiviDB indexes an empty stored TAG is unmeasured.

The test design holds up:

  • In the exact-count fixture each value has its own document count (12, 14, …, 38), and the test asserts the exact ids, not just recall. Because it queries with kividb_caps::tag_literal itself, it can't fall into the PR Add kividb engine #203 failure where a test reimplements the logic it guards.
  • Every escape-fixture value is also queried on its own at least once, so one mis-encoded value costs a whole query (≥0.1 recall on 10 queries), which the 0.95 floor catches.
  • The pre-existing test_binary_kividb_uuid already exercises escaped hyphens live on the 1.1.2 pin, so the flipped kividb: metadata filtering broken against real KiviDB (TAG escaping + atomic tags); FT.CREATE SEPARATOR fixed #205 regression guard (uuid_hyphens_are_escaped) has live coverage, not just a unit assertion.
  • The mutation list in the body (escaping reverted → 3 of 26 red live, paren fixture at recall 0.630; plus the unit mutations) is what this repo's bar asks for.

Worth fixing before merge

Nothing found.

Optional / follow-up (not blockers)

  1. The arxiv corpus check ran on 1.0.5, not the CI pin. The full-corpus check (2,123,402 submitter values, every escaped query value returning its expected count) ran on 1.0.5. That 1.0.5 handles escapes the same way as 1.1.2 rests on the 146-value probe. The README is honest about this (README.md "Which shipped datasets this excludes"). Still, the one shipped dataset this PR unblocks has its end-to-end evidence on a version that is now outside the support gate. If the Redis-wire TAG semantics (; in scalar values, case folding) + deferred KiviDB test/CI items from #375 #377 re-measure pass happens anyway, a 1.1.2 re-run of that check would close the gap.
  2. Empty TAG values now differ from redis.rs. The removed comment in build_match_any said empty strings were skipped "same as redis.rs". KiviDB now hard-errors on them while redis.rs presumably still skips them. That is deliberate and documented here, but it's the same kind of cross-engine question Redis-wire TAG semantics (; in scalar values, case folding) + deferred KiviDB test/CI items from #375 #377 already tracks (;, case folding). It's worth one line there so the Redis-wire engines stay comparable in one place.
  3. Wording nit on tag_literal's doc. It says the !tags_supported arm "also covers a string condition on a field the schema does not declare as a TAG". That is true for strings only: the bool exact-match path (@f:{true}) is still emitted on an unsupported server, and unsupported_server_refuses_any_tag_value_and_nothing_else pins that on purpose. It's harmless, because true/false need no escaping and validate_dataset_support refuses a bool-declared dataset first. The doc could just say "string".

Squash-merging as the body asks, keeping the Co-authored-by trailer, avoids misattributing b7dda91 in blame.

Net: a careful, well-measured fix to a silent wrong-recall on the one shipped dataset it affects, with live exact-count coverage on the pinned image. Nothing here should block it; the items above are follow-up material.

…s on `,`

#375 automated review, optional item 2: `search()` checked the server
version and that the index exists, but not how an existing index
declares its TAG fields. An index built bare on a >= 1.0.5 server (by a
binary predating this PR, i.e. master today, or by hand) reports
`SEPARATOR ,` and under-matches every comma-bearing value; reused with
`--skip-upload` it would be measured without a word (the mutation table
measured that declaration at recall 0.270).

- kividb_caps: `ft_info_tag_separators` reads each TAG attribute's
  `SEPARATOR` from FT.INFO (KiviDB 1.0.5 lists `attributes` and reports a
  bare TAG as `SEPARATOR ,`, measured; 1.0.2 has no `attributes`, -> None)
  and `check_index_tag_separators` requires `;` for every schema TAG field
  on Separator servers. A >= 1.0.5 index that reports no attributes is
  unverifiable and refused; Atomic servers have nothing to check.
- kividb.rs `search()`: runs it after `ensure_index_exists`, in the same
  pre-`read_queries()` block, on both the normal and `--skip-upload` paths
  (on the normal path it asserts the server honoured `configure()`'s
  declaration: AGENTS.md "assert the SERVER reports it").
- indexing_wait: `ft_info_pairs` / `value_text` made pub(crate) for reuse.
- Live test `test_kividb_skip_upload_refuses_a_stale_bare_tag_index`:
  upload the comma fixture with --keep-data, rebuild the index bare over
  the kept keys -> --skip-upload refused naming `author` and `","`, no
  search result written; rebuild with `SEPARATOR ;` -> accepted at recall
  >= 0.9 (positive control). Skips its body on a pre-1.0.5 server.
- tests/common: `run_binary_capture_extra`.

integration_kividb: 23/23 on v1.0.5-full and on v1.0.2-full (atomic).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Streamed every shipped dataset with a TAG-typed field: h-and-m has a
comma in 99 query filter values (and ~17k corpus values), so bare TAGs
on 1.0.5 would silently under-match there; no shipped query value
contains `;` (corpus: 12 arxiv submitters, 10 MS MARCO urls, none
queried) -- cross-engine follow-up in #377. Also notes the FT.INFO
declaration check on reused indexes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@fcostaoliveira

Copy link
Copy Markdown
Collaborator Author

Reply to the automated review above (pass 1, on efbe527). Follow-ups pushed in 7f7c268.

1. A ; inside a scalar filter value under-matches on >= 1.0.5. Agreed it's real, and checked against shipped data as you asked. I streamed every shipped dataset with a TAG-typed field (h-and-m, arxiv, random-match-keyword-100, MS MARCO 100K), corpus and queries:

  • ; in a query filter value: 0 everywhere (h-and-m ~10k values, arxiv 6.6k, random-keyword 16.6k; MS MARCO has no filtered vector queries).
  • ; in the corpus: 12 arxiv submitter values (HTML entities such as &#322;) and 10 MS MARCO urls, none of them queried.

The corpus half (a stored a;b over-matching @f:{a}) can't be caught query-side, and it's identical on Redis/Valkey/Dragonfly, which all declare SEPARATOR ;. So I filed the guard as a cross-engine issue, #377 (query-side refusal + upload-side check + fixtures), rather than special-casing KiviDB's query side here. The same scan shows why the comma fix matters: h-and-m has a comma in 99 query filter values (section_name, garment_group_name), which would silently under-match on 1.0.5 with bare TAGs. Both numbers are now in the README (7f7c268).

2. --skip-upload doesn't check how an existing index was declared. Fixed (1b6138d). KiviDB 1.0.5's FT.INFO lists attributes and reports a bare TAG as SEPARATOR , (measured; 1.0.2 has no attributes). The new kividb_caps::ft_info_tag_separators + check_index_tag_separators require ; on every schema TAG field on a >= 1.0.5 server. On such a server, an index that reports no attributes is refused as unverifiable; pre-1.0.5 servers have nothing to check. search() runs it after ensure_index_exists, still before read_queries(), on both paths. On the normal path it also asserts the server honoured configure()'s declaration.

  • Live test test_kividb_skip_upload_refuses_a_stale_bare_tag_index: it uploads the comma fixture with --keep-data and rebuilds the index bare over the kept keys. --skip-upload is then refused, naming author and ",", and no search result is written. As a positive control, rebuilding with SEPARATOR ; is accepted at recall >= 0.9.
  • Mutations (restored after): check not called → that live test red; any separator accepted → unit + live red; no-attributes accepted on >= 1.0.5 → unit red.

3. expected_tag_semantics() defaults to separator. Kept as is, by design, as you note. The default matches the compose kividb service, and the failure message names KIVIDB_EXPECT_TAG_SEMANTICS and what the server reported.

Verification at 7f7c268: make agent-check exit 0; integration_kividb 23/23 on v1.0.5-full and 23/23 on v1.0.2-full (KIVIDB_EXPECT_TAG_SEMANTICS=atomic).

fcostaoliveira and others added 2 commits October 29, 2026 12:40
… and the FT.INFO check

#375 review pass 2, optional item 1: `"keyword" | "uuid" | "bool"` was
written out in `create_index`, `validate_dataset_support` and
`tag_field_names`. A TAG type added to `create_index` but not to
`tag_field_names` would drop out of the FT.INFO declaration check, so a
stale bare index on that field would pass `--skip-upload` silently.
`is_tag_type` is now the one definition, and
`tag_field_names_are_exactly_the_tag_typed_fields` pins the set.

The Co-authored-by trailer below credits #326's author in the default
squash message (this repo squashes with COMMIT_MESSAGES, and ce46925
itself is authored as Claude).

Co-authored-by: murtazayusuf <65263321+murtazayusuf@users.noreply.github.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
#375 review pass 2, optional item 2: under SEPARATOR ; the risk from a
stored `a;b` is a query equal to a FRAGMENT. Streamed arxiv again: none
of its 2,928 distinct submitter query values equals any of the 20
fragments of the 12 `;`-bearing stored values (case-insensitive, trimmed,
as a TAG matches). MS MARCO has no filtered vector queries; h-and-m and
random-keyword store no `;`.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@fcostaoliveira

Copy link
Copy Markdown
Collaborator Author

Reply to the automated review (pass 2, on 7f7c268). Follow-ups pushed in 945da24.

1. TAG types written out in three places. Fixed (bc48753). is_tag_type is now the single definition, used by create_index, validate_dataset_support and tag_field_names, which scopes the FT.INFO check. tag_field_names_are_exactly_the_tag_typed_fields pins the set. Mutations: letting tag_field_names drift back to its own literal without bool → that test red; dropping bool from is_tag_type → it and unknown_version_refuses_every_tag_field_and_nothing_else red.

2. The ; note checked the wrong direction. You're right that the risk is a query equal to a fragment. I re-streamed arxiv: none of its 2,928 distinct submitter query values equals any of the 20 ;-fragments of the 12 stored values, compared case-insensitively and trimmed, as a TAG matches. MS MARCO has no filtered vector queries, and h-and-m and random-keyword store no ;. README and #377 updated (945da24).

3. Squash attribution. Thanks for flagging this. It needed more than the PR body. This repo squashes with COMMIT_MESSAGES, and ce46925 is authored as Claude <noreply@anthropic.com>, so the body's trailer would not have reached the squash commit. bc48753's message now carries Co-authored-by: murtazayusuf <65263321+murtazayusuf@users.noreply.github.com>, so the default squash text includes it, with no force-push needed. Whoever merges should still glance at the squash text.

Verification at 945da24: make agent-check exit 0; integration_kividb 23/23 on v1.0.5-full and 23/23 on v1.0.2-full.

@fcostaoliveira

Copy link
Copy Markdown
Collaborator Author

Reply to the automated review (pass 3, on 945da24). Nothing pre-merge was raised. On the optional items:

  1. Comma fixture on Redis/Valkey/Dragonfly. Agreed, and it's cross-engine, so I added it to Redis-wire TAG semantics (; in scalar values, case folding) + deferred KiviDB test/CI items from #375 #377 (item 4) alongside the ; guard rather than widening this PR.
  2. --skip-upload separator readback for the Redis family. Also added to Redis-wire TAG semantics (; in scalar values, case folding) + deferred KiviDB test/CI items from #375 #377 (item 5), noting the lower risk you describe.
  3. Hardcoded idx:{name} in the stale-index test. Left as is. It follows the suite's existing convention (test_kividb_coexistence_skip_upload hardcodes idx:kividb-co-a the same way), and as you note a non-default KIVIDB_INDEX_NAME makes it fail loudly, not wrongly.

No code changes for this pass; head stays 945da24.

@murtazayusuf

Copy link
Copy Markdown

Thanks a lot @fcostaoliveira for carrying #326 forward, and for the thorough follow-ups.

One request before merge: could CI use KiviDB 1.1.2, our current stable release, instead of 1.0.5?

  • tests/docker-compose.test.yml: quay.io/kividbio/kividb:v1.0.5-full → kividb/kividb:v1.1.2-full (Docker Hub), or ghcr.io/kividbio/kividb:v1.1.2-full. Quay no longer gets new images since 1.1.0, so there's no 1.1.2 there. Docker Hub and GHCR carry every release.
  • The >= 1.0.5 gate stays right: 1.1.2 splits TAGs on SEPARATOR exactly as 1.0.5 does. Please avoid 1.1.1 if it ever comes up; it was withdrawn.
  • Keeping the v1.0.2-full legacy job for the pre-1.0.5 branch makes sense. That image stays on Quay.
  • If it isn't much work, it would be nice for the README's "Re-measured" section to say 1.1.2 rather than 1.0.5. If the probes come out the same, a one-line note is enough.

If it's easier, we can push that change to our fork (kividbio/vector-db-benchmark) for you to pick up. Just say which you prefer.

KiviDB >= 1.0.5 honours RediSearch backslash escapes, so the unchanged
raw emitter sent values that silently matched the wrong documents there.
Measured on 1.0.5 with the shipped arxiv `submitter` query values:

- 56 distinct values with `\` returned 0 docs (`J\"org Hennig`: 0, 31 expected);
- 13 values with `{`/`}` were a syntax error, and search() drops a failed
  query with only a log line.

77 of arxiv's 10,000 queries were affected. On 1.0.2 the raw form is right
and escapes match nothing, so the encoding now follows the server:

- parse_conditions() takes the server's TagSemantics (from search()'s
  existing INFO read);
- Atomic (< 1.0.5): raw, as before;
- Separator (>= 1.0.5): a backslash before every ASCII punctuation
  character and space. Measured exact for all 32 plus space, bare, grouped
  and AND'd; non-ASCII punctuation is exact unescaped;
- unknown: a hard error for any TAG value.

tag_value_problem() replaces tag_value_offender() and refuses, per version,
what no encoding expresses:

- every version: leading or trailing whitespace (trimmed by the query parser);
- < 1.0.5: `| @ ( )`, as before;
- >= 1.0.5: `;` (the declared separator), a trailing `*` (a prefix wildcard
  even escaped) and a leading `$` (a parameter reference even escaped).

`| @ ( )` are now exact on >= 1.0.5. That unblocks arxiv-filters, which
aborted in search() on `Francisco (Paco) Guinea`. No shipped query value hits
any remaining refusal.

Tests:
- per-version emission unit tests;
- two live fixtures in both CI suites, with `--fail-on-dropped-queries`:
  - backslash, braces, quote and comma values next to their wrong-encoding
    decoys (recall 1.0 on both versions);
  - `( ) | @` values (recall 1.0 on 1.0.5, refused on 1.0.2).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

Docker Build Validation

Platforms tested:

  • linux/amd64 (built and tested)
  • linux/arm64 (build validated)

Tests performed:

  • --help command
  • --describe datasets command
  • Integration test (random-100 dataset + Redis)
  • Multi-platform build validation (amd64 + arm64)

README and kividb.rs:
- The filter emitter is no longer "correct on every version": the TAG value
  encoding follows the server (raw before 1.0.5, escaped from 1.0.5).
  Divergence 2 now says backslashes and braces round-trip raw only before
  1.0.5. The doc lists the measured hard errors for each version.
- Shipped exposure, for every downloadable dataset with a TAG field:
  - 77 arxiv queries contain `\` (69 distinct values), 14 of them also
    braces, and 4 contain parentheses;
  - nothing else is exposed, and no shipped query value hits a remaining
    hard error.
- "2 of the 62" is now true on >= 1.0.5: arxiv runs. The unknown-version
  refusal of all 12 TAG datasets is in the table and the note, and
  "exclude the three" now reads "two (three on < 1.0.5)".
- TAG matching is case-insensitive on 1.0.2 as well as 1.0.5 (measured), as
  in redis.rs's layout. KiviDB's numbers do not shift across the boundary,
  and the over-match is noted with the reviewer's counts.
- The table header says that row 10 is index-side. The FT.INFO module doc is
  scoped to 1.0.2.

Version gate and FT.INFO read-back (k1, k2 and k3 optional items):
- The unknown-version remedy no longer assumes the field is absent.
- check_index_tag_separators names the reported kividb_version in every
  refusal, tells the `Smith, John` story only for a `,` separator, and its
  `Some(None)` and malformed-FT.INFO paths are tested.
- `1.0.5+` (empty build metadata) is refused.
- FT.INFO parse errors carry the engine and index name.

Tests and CI (k5, k6 and k7 optional items):
- New legacy-only live test: `--skip-upload` over a hand-built `labels TAG`
  index on 1.0.2 is refused by search()'s version check, the only guard on
  that path.
- The pre-1.0.5 CI steps run `if: !cancelled()`, so one round shows both
  suites.
- AGENTS.md lists `make integration-test-kividb-legacy`.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

Docker Build Validation

Platforms tested:

  • linux/amd64 (built and tested)
  • linux/arm64 (build validated)

Tests performed:

  • --help command
  • --describe datasets command
  • Integration test (random-100 dataset + Redis)
  • Multi-platform build validation (amd64 + arm64)

1 similar comment
@github-actions

Copy link
Copy Markdown

Docker Build Validation

Platforms tested:

  • linux/amd64 (built and tested)
  • linux/arm64 (build validated)

Tests performed:

  • --help command
  • --describe datasets command
  • Integration test (random-100 dataset + Redis)
  • Multi-platform build validation (amd64 + arm64)

@murtazayusuf

Copy link
Copy Markdown

@fcostaoliveira, an update to my earlier comment: we've gone further and moved the KiviDB support to 1.1.2 only. The gate is now 1.1.2 and the 1.0.2 legacy job is gone. Your commits are carried unchanged in #381, with one commit of ours on top: details there. Please merge whichever you prefer. If you'd rather keep this PR, cherry-picking 62ada3a onto it gives the same result, and we'll close #381. Thanks again for all the follow-up work.

…alued

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
murtazayusuf and others added 2 commits October 29, 2026 16:13
CI's KiviDB service moves to kividb/kividb:v1.1.2-full (Docker Hub; also
ghcr.io/kividbio/kividb). The tool now supports and tests only the current
stable release for TAG fields.

- kividb_caps: TAG fields are declared `TAG SEPARATOR ;` on KiviDB >= 1.1.2
  (TAG_MIN_VERSION; a pre-release is gated on its core version, so
  1.1.2-rc1 counts). An older server, or one whose kividb_version cannot
  be read, gets no TAG declaration: every dataset that declares a TAG
  field is refused in configure(), multi-valued `labels` included, and
  the message names what INFO server reported. Vector-only and
  numeric/text-only datasets still run. The atomic-TAG branch is removed
  with the job that tested it.
- Remove the kividb-legacy compose service, the
  integration-test-kividb-legacy target, its CI step and
  KIVIDB_EXPECT_TAG_SEMANTICS.
- docker-compose: the 1.1.2 image is distroless (no shell, no
  redis-cli), so the bash /dev/tcp healthcheck cannot run and
  `up --wait` failed. Drop it, as for Chroma; wait_for_kividb() polls
  PING for readiness.
- README KiviDB note: the divergence probes re-run on 1.1.2 against Redis
  Stack 7.4.7 and Redis 8.8.0 on the same fixture; every probe matches
  except GEO (still absent). FT.INFO on 1.1.2 reports num_docs,
  percent_indexed and TAG attributes with SEPARATOR; RESP3 works.
- Module docs, comments and unit tests describe behaviour as of 1.1.2.
…ure on 1.1.2

On top of 62ada3a (the 1.1.2 retarget, cherry-picked from #381):

- Move tag_literal / escape_tag_value / tag_value_problem from the
  engine's kividb_filter into kividb_caps (library), per AGENTS.md rule 3.
  The engine calls them unchanged. The live suite can now query the
  engine-built index with exactly the clause the engine sends.
- New live fixture test_binary_kividb_tag_values_exact_counts. Values with
  `\`, `{`, `}`, `"`, `,`, `(`, `)` and `|` each sit next to the decoy a
  wrong encoding hits, and each is stored on a distinct number of
  documents (12..38). The engine runs the dataset with
  --fail-on-dropped-queries --keep-data. Then each value is queried bare
  and inside a group with kividb_caps::tag_literal, and the test asserts
  the exact document ids.
- The hard-error set re-measured on kividb/kividb:v1.1.2-full (CI probe,
  #382, closed). Escaped, all 32 ASCII punctuation characters plus space
  are exact at the start, middle and end of a value, bare, grouped and
  AND'd. That includes `( ) | @ \ { }`, so parentheses need no hard
  error. What remains:
  - a trailing `*`, a prefix wildcard even escaped (`zz\*` matched 73);
  - a leading `$`, a parameter (`No such parameter`);
  - `;`, the separator;
  - leading or trailing whitespace, trimmed on both sides.
  Raw, 22 of 146 probe values were wrong (`a\b` -> `ab`,
  `J\"org Hennig` -> `J"org Hennig`, `x|y` -> `x` + `y`, and `}` or a
  leading `"` gave syntax errors).
- Docs and unit tests carry the 1.1.2 numbers.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…st in the cherry-pick)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@fcostaoliveira fcostaoliveira changed the title feat(kividb): filter multi-valued labels on KiviDB >= 1.0.5; CI on v1.0.5-full (supersedes #326) feat(kividb): filter multi-valued labels and escape TAG values on KiviDB 1.1.2; CI on v1.1.2-full (supersedes #326, #381) Oct 29, 2026
fcostaoliveira added a commit that referenced this pull request Oct 29, 2026
…381)

Multi-valued TAG labels on KiviDB (SEPARATOR ;), with the FT.INFO separator
read-back on reused indexes. TAG support is gated on KiviDB >= 1.1.2: older
servers, or ones whose version can't be read, refuse TAG datasets and name
the version reported. CI runs kividb/kividb:v1.1.2-full from Docker Hub.

Supersedes #326. Follow-up: #375 (TAG value escaping on 1.1.2).

Co-authored-by: fcostaoliveira <fcostaoliveira@users.noreply.github.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
fcostaoliveira and others added 2 commits October 29, 2026 17:12
Final review pass on ae1027d (f1-f7):

- tag_value_problem now hard-errors on an empty value and on any interior
  whitespace other than a space or a control character (tab, newline, CR,
  VT, FF, U+00A0, U+3000, U+2028, ...). Neither was measured on 1.1.2 and
  escape_tag_value does not escape them. tag_clause and match_any no longer
  turn an empty value into a never-match; both go through tag_literal. A
  scan of every shipped TAG query value (h-and-m, arxiv, three
  random-keyword sets, tenants-100) finds none of these shapes.
- KividbVersion::prerelease and parse_kividb_version were test-only: the
  field is dropped (the suffix is still validated) and the helper moved
  into the tests.
- Docs: stale escape_tag_value path; "no declaration is known to be right"
  replaced by the measured 1.0.5-1.0.6 equivalence; stored `;` is not
  inert; README hard-error list, the scalar-TAG support narrowing, and the
  arxiv submitter value count (2,123,402 values across 2,138,591 docs).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…alued

#381 squashed 945da24 + 62ada3a, both already on this branch (62ada3a as
b7dda91). Conflicts in README.md, kividb.rs, kividb_caps.rs,
tests/common/mod.rs and tests/integration_kividb.rs resolve to this
branch's newer versions; the merged tree is identical to 8e0c582. Against
master the PR is now only the follow-up: TAG value escaping, the
exact-count fixture, doc fixes and the final-review batch.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@fcostaoliveira fcostaoliveira changed the title feat(kividb): filter multi-valued labels and escape TAG values on KiviDB 1.1.2; CI on v1.1.2-full (supersedes #326, #381) kividb: exact TAG filter values on 1.1.2 (escaping + live fixture), follow-up to #381 Oct 29, 2026
@github-actions

Copy link
Copy Markdown

Docker Build Validation

Platforms tested:

  • linux/amd64 (built and tested)
  • linux/arm64 (build validated)

Tests performed:

  • --help command
  • --describe datasets command
  • Integration test (random-100 dataset + Redis)
  • Multi-platform build validation (amd64 + arm64)

1 similar comment
@github-actions

Copy link
Copy Markdown

Docker Build Validation

Platforms tested:

  • linux/amd64 (built and tested)
  • linux/arm64 (build validated)

Tests performed:

  • --help command
  • --describe datasets command
  • Integration test (random-100 dataset + Redis)
  • Multi-platform build validation (amd64 + arm64)

…im; scan and wording nits

- README: the pre-1.1.2 refusal arrived with #381, and the geo datasets
  declare no TAG field (they already aborted before #219).
- README: hard-error wording reads unambiguously (control characters are
  refused); the scan note covers synthetic-filter-32.
- tests/common: the fixture doc says the queries cycle through N_QUERIES.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@fcostaoliveira

Copy link
Copy Markdown
Collaborator Author

Replies to the 17:14 automated pass on 96ce29d, plus the last reviewer round. The doc fixes are in 16868cb: README and tests/common/mod.rs only, no src/ change.

  1. Empty match_any arm vs redis.rs: tracked in Redis-wire TAG semantics (; in scalar values, case folding) + deferred KiviDB test/CI items from #375 #377 C. We keep the hard error on KiviDB, because an empty value is unmeasured there.
  2. Exact-count fixture in hybrid form: tracked in Redis-wire TAG semantics (; in scalar values, case folding) + deferred KiviDB test/CI items from #375 #377 C. Today the hybrid shape is covered by recall 1.0 in the same live runs.
  3. Scan coverage: the README now says synthetic-filter-32 only emits fixed colour literals, so it can't hit a refusal.
  4. The --skip-upload version-check live test: agreed. It stays deferred, because a 1.1.2-only CI can't reach that branch live. The unit tests cover both sides of TAG_MIN_VERSION.
  5. "ten queries": the doc now says the queries cycle through the N_QUERIES (10) targets.

Also fixed from the reviewer round:

Unicode format characters (U+200B, U+FEFF, …) aren't measured on 1.1.2 and aren't refused. No shipped value contains one; this is noted for #377.

🤖 Generated with Claude Code

@github-actions

Copy link
Copy Markdown

Docker Build Validation

Platforms tested:

  • linux/amd64 (built and tested)
  • linux/arm64 (build validated)

Tests performed:

  • --help command
  • --describe datasets command
  • Integration test (random-100 dataset + Redis)
  • Multi-platform build validation (amd64 + arm64)

@fcostaoliveira
fcostaoliveira merged commit 3a397dd into master Oct 29, 2026
38 checks passed
fcostaoliveira added a commit that referenced this pull request Oct 29, 2026
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants