Repository navigation
tmp: #375 mutation check, TAG escaping reverted (throwaway, do not merge) - #383
Closed
fcostaoliveira wants to merge 18 commits into
Closed
fcostaoliveira wants to merge 18 commits into
fcostaoliveira wants to merge 18 commits into
Conversation
….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>
…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>
… 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>
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>
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>
…alued Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Collaborator
Author
|
Mutation confirmed: with escaping reverted, the KiviDB job is red (3 of 26 failed: 🤖 Generated with Claude Code |
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.
Mutation check for #375:
escape_tag_valuereturns the value unchanged. The KiviDB integration job is expected to go red. Will be closed and the branch deleted once that is confirmed.🤖 Generated with Claude Code