docs(search): measure the include_derived blast radius (09/18 c-03) - #1703
Merged
Merged
Conversation
Contributor
|
Claude Code Review — skipped: PR author 'arkash20' is not a public member of the 'caura-ai' org |
Contributor
Author
|
@Eldad-Caura please approve |
Eldad-Caura
previously approved these changes
Sep 23, 2026
arkash20
force-pushed
the
docs/pm-0918-c-03-derived-row-blast-radius
branch
from
September 23, 2026 17:12
7db9e4b to
e52bd7d
Compare
Contributor
|
Claude Code Review — skipped: PR author 'arkash20' is not a public member of the 'caura-ai' org |
arkash20
force-pushed
the
docs/pm-0918-c-03-derived-row-blast-radius
branch
from
September 23, 2026 20:28
e52bd7d to
ea88fe9
Compare
Contributor
|
Claude Code Review — skipped: PR author 'arkash20' is not a public member of the 'caura-ai' org |
arkash20
force-pushed
the
docs/pm-0918-c-03-derived-row-blast-radius
branch
from
September 23, 2026 20:42
ea88fe9 to
900e4d6
Compare
Contributor
|
Claude Code Review — skipped: PR author 'arkash20' is not a public member of the 'caura-ai' org |
arkash20
added a commit
that referenced
this pull request
Sep 23, 2026
…8 c-03) (#1708) Implements the decision taken on pm-0918-c-03: **option (a)** — a request flag defaulting to today's behaviour, plus a per-tenant default. The evidence behind the choice is in #1703 (docs-only, merges independently). A70's fan-out writes a short single-claim child per extracted fact, and those children stay retrievable alongside their parent. On the store that prompted this they were ~28% of returned rows — a caller asking for 50 memories got 36 memories and 14 fragments, worth 79.8% vs 82.2% on that harness. Filtering them client-side does not work, and that is the design constraint: **dropping rows after the response distorts `top_k`.** Ask for 50, drop 14, get 36, then over-fetch and guess. So the filter runs server-side and *before* the trim, where the existing 2× `SEARCH_OVERFETCH_FACTOR` absorbs it — at ~28%, 100 candidates become ~72 survivors and all 50 slots still fill. ## Three layers, request beats tenant beats global | layer | where | default | |---|---|---| | `include_derived` on the request | `POST /search`, `POST /recall` | unset (tri-state) | | `search.include_derived` | per tenant, `PUT /settings` | unset | | `INCLUDE_DERIVED_DEFAULT` | `constants.py` | `True` — **unchanged** | The global default deliberately does **not** move. `/search` is in the frozen broker subset and a default flip is a semantic change `oasdiff` cannot see, so it needs a `BREAKING CHANGE:` trailer and a second store's evidence. A tenant that wants that behaviour today sets `search.include_derived=false` and gets it for its own store. ## The predicate is a conjunction, and both halves are load-bearing ``` parent_memory_id IS NOT NULL AND source = 'atomic_fact_fanout' ``` `parent_memory_id` alone also catches **auto-chunk children, which are not redundant** — the parent holds the whole document but carries *one* embedding over all of it, so those children are the only vectors that can match a specific passage. Excluding them would surface days later as "long documents stopped being findable" with nothing pointing at this filter. And `source` alone is forgeable: it is deliberately absent from `PLATFORM_ONLY_KEYS` because ingest writes it too. The argument sits in a comment at the predicate, not only in the doc. Applied in `PostFilterResults` **and** in the legacy search path from one shared helper in `search_trim.py`, so the `_USE_PIPELINE_SEARCH` rollback lever cannot silently revert it — two ranking features already shipped pipeline-only for exactly that reason. ## Not repeating the c-04 defect `search.include_derived` is registered in `DEFAULT_SETTINGS`, in `_LEAF_TYPES` as `bool`, **and** on the resolver. On pm-0918-c-04 the switch existed only as a `ResolvedConfig` property: `_check_keys` rejected every write, `PUT /settings` answered 422, and the resolver served the default anyway — so it read as shipped. `test_the_tenant_default_survives_a_real_settings_put` goes through the **HTTP route**, not a constructed `ResolvedConfig`, because constructing it directly is what hid that. **Verified by removing the `DEFAULT_SETTINGS` entry and re-running**, which fails with the exact c-04 error: ``` AssertionError: PUT failed — the knob is unsettable: {"detail":"Unknown settings key(s): ['search.include_derived']"} ``` The `bool` leaf type earns its place separately: `"false"` is a truthy string, so without it a tenant that switched derived rows off would resolve to ON while the dashboard rendered their "off" back to them. ## Verification - 20 new tests, all passing. Precedence is tested at all three layers including the both-supplied case in **both** directions, and the global default is pinned with a test whose failure message says to use a breaking-change trailer rather than edit the assertion. - Full suite: **93 failures before and after**, 7,754 → 7,774 passing (+20). The 93 are pre-existing local failures — baselined by running the identical suite on unmodified `origin/main`. - `ruff check` + `ruff format --check` clean. `mypy` reports zero errors in the changed files (the 142 in the transitive graph are pre-existing and identical at baseline). - Broker baseline regenerated in its own commit; `--check` passes and the pinned `oasdiff` image reports *"No breaking changes to report, but the specs are different"*, exit 0. ## MCP: a limitation, and a silent drop that is now its own row This PR does **not** add `include_derived` to the MCP tools — that pulls in the 40+ location tool-surface checklist, and the tenant setting covers MCP callers. `/recall` *was* wired, and the line between the two is worth stating so it does not read as inconsistent: `RecallRequest` subclasses `SearchRequest`, so a caller **can** send the field to `/recall` whether or not the route reads it — accepting and ignoring it is silent divergence, the class of ax-0917-m-19 / m-16 / oss-0922-m-03. MCP does not expose the field at all, which is a limitation rather than a divergence. **That last claim was checked rather than assumed, and it did not survive.** Probing `mcp.call_tool("caura_recall", ...)` through the real dispatch: | argument sent | what happened | |---|---| | `include_derived: false` | `search_memories` received `include_derived=<ABSENT>` | | `bogus_param_xyz: 2` | no error, no warning | | `limit: 2` | no error; response reports `effective_top_k: 5` | Every one returns a normal success envelope with `isError` absent. The argument is dropped during the SDK's argument binding, before the handler. So **MCP is a silent-drop surface too** — and `limit` silently returning the default 5 is ax-0917-h-05's original defect, still unfixed on the surface agents actually use. That is broader than this row (it is the argument-binding path, so all 12 tools) and is filed as **`oss-0923-h-01`** rather than widening this PR. The field description now says so at the point of use: REST only, use the tenant setting from MCP, and an unknown MCP argument is dropped silently rather than reported. ## For Arkady — flipping PersonaMem's tenant Not run here: this is a live-data action on a real tenant. Idempotent, replace the placeholder: ```bash curl -sS -X PUT "https://memclaw.dev/api/v1/settings?tenant_id=<PERSONAMEM_TENANT_ID>" \ -H "X-API-Key: $CAURA_API_KEY" \ -H 'Content-Type: application/json' \ -d '{"search": {"include_derived": false}}' ``` Read it back with `curl -sS "https://memclaw.dev/api/v1/settings?tenant_id=<PERSONAMEM_TENANT_ID>" -H "X-API-Key: $CAURA_API_KEY" | jq .search.include_derived` — expect `false`. Settings are cached per tenant, so allow for the cache TTL before re-measuring. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Signed-off-by: Arkady Mankovsky <arkash20@gmail.com>
arkash20
force-pushed
the
docs/pm-0918-c-03-derived-row-blast-radius
branch
from
September 23, 2026 20:56
900e4d6 to
c992a3d
Compare
Contributor
|
Claude Code Review — skipped: PR author 'arkash20' is not a public member of the 'caura-ai' org |
pm-0918-c-03 is an API decision, not a bug fix. This adds the evidence to take it and changes no search behaviour. The finding that matters most is that options (a) "request flag" and (c) "exclude by default" are the SAME implementation and one boolean — (c) needs (a)'s flag to opt back in with. The decision is a default value, reversible in one line in either direction, not a choice between designs. Three things the analysis turned up that the row did not have: - The filter predicate has to be a conjunction of parent_memory_id AND source='atomic_fact_fanout'. parent_memory_id alone also catches auto_chunk children, which are not redundant — they are the only sub-document vectors a long document has, since the parent carries one embedding over its whole content. source alone is forgeable: system_metadata.py deliberately leaves `source` out of PLATFORM_ONLY_KEYS. source_uri=null is incidental, and a filter using it would drop 19,622 ordinary rows on the dev corpus. - The local corpora cannot price this, and the document says so rather than quoting a rate. `caura` — the only database written after A70 shipped — has zero derived rows. `memclaw`'s 1,486 children all come from 703 benchmark, fast-mode parents written on one day three days before A70. Fan-out children do not inherit the parent's `benchmark` marker, so a provenance query on the child reports all 1,486 synthetic rows as real traffic; the script joins through the parent instead. - POST /search is in the frozen broker subset, and flipping a default is a semantic change with no schema movement: oasdiff passes it silently. Only the BREAKING CHANGE trailer and kind/breaking label stand in the way, and both are human gates. benchmark/pm_c03_derived_row_population.py is re-runnable against any store and carries an ORPHAN category for rows with a parent link and an unrecognised source, so a future writer of that shape is a decision rather than an accident. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Arkady Mankovsky <arkash20@gmail.com>
arkash20
force-pushed
the
docs/pm-0918-c-03-derived-row-blast-radius
branch
from
September 23, 2026 21:12
c992a3d to
840c7c3
Compare
Contributor
|
Claude Code Review — skipped: PR author 'arkash20' is not a public member of the 'caura-ai' org |
… flagged The blast-radius measurement names the `memclaw` database in four places: a usage line in the script, the table row reporting what was measured there, the sentence saying that store holds the only fan-out population, and the reproduce command. The ratchet counted these as minting a new legacy name. They are not. That database exists locally and literally bears the name, and three of the four cannot be reworded and stay true: you cannot report which store a figure came from without naming the store, and a pasteable command against a differently-named database does not run. That is the floor test rather than the reword test. The file already carried one accepted floor marker for the same reason (the running container's name at :146), so this follows a precedent set in the same file rather than inventing one. Ratchet now reports "Gate passes: no new lines currently fail it". Not fixed here, and pre-existing on this branch's base: ruff EXE001 and PLW1510 in this script. Neither is reached by CI, which lints core-api, core-storage-api, common and core-operations but not benchmark/. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Arkady Mankovsky <arkash20@gmail.com>
Contributor
|
Claude Code Review — skipped: PR author 'arkash20' is not a public member of the 'caura-ai' org |
erni-a
approved these changes
Sep 23, 2026
Merged
Eldad-Caura
pushed a commit
that referenced
this pull request
Sep 24, 2026
🤖 I have created a release *beep* *boop* --- <details><summary>backend: 3.19.0</summary> ## [3.19.0](backend-v3.18.1...backend-v3.19.0) (2026-09-24) ### Features * **search:** let a caller or a tenant exclude fan-out children (09/18 c-03) ([#1708](#1708)) ([1c98ce1](1c98ce1)) ### Bug Fixes * **api:** make the 504 name the layer that ate the request budget (ax-0917-h-01/h-02) ([#1707](#1707)) ([68d49fa](68d49fa)) * **installer:** verify TLS unless asked to trust on first use ([#1711](#1711)) ([cab9eed](cab9eed)) ### Documentation * **c-01:** measure what a deferred title does to FTS rank and ordering ([#1704](#1704)) ([2e7937a](2e7937a)) * **comments:** give the a57 findings doc a findable path ([#1709](#1709)) ([ea4d066](ea4d066)) * **search:** measure the include_derived blast radius (09/18 c-03) ([#1703](#1703)) ([ee482a9](ee482a9)) </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Signed-off-by: release-please[bot] <release-please[bot]@users.noreply.github.com> Co-authored-by: caura-deploy-bot[bot] <265395343+caura-deploy-bot[bot]@users.noreply.github.com>
arkash20
added a commit
that referenced
this pull request
Sep 24, 2026
…public repo `caura-ai/caura` is public. Two places asserted a 79.8% -> 82.2% (+2.4pp) improvement as a fact about this feature, with no provenance and no conditions: the findings doc and the test docstring. Both came from my own #1703 and #1708 on 2026-09-23. The figures come from the PersonaMem store, which is the subject of pm-0918-c-01 and pm-0918-c-05 -- a store whose headline number has not reproduced across two later runs. If that number is untrustworthy because of store state, this one inherits the same doubt. We were publicly claiming an improvement measured on ground we are simultaneously arguing internally that we cannot stand on. Three qualifications, none of which was stated: * It was measured CLIENT-side, before the server-side exclusion that refills `top_k` existed -- roughly 24 rows deleted from 85 and not backfilled, against a thinner context than a store that never had them. So +2.4pp is a FLOOR, not an unbiased estimate of what the shipped filter is worth. * `TrackRecalls` bumps `recall_count` on every returned row, children included, and it feeds the score. Query-time filtering cannot undo a counter children already accrued. * It is a within-run delta on one named session, `caura-bulk-2k-top50-sess2` -- not comparable to the record run, and chaining it with that run's figure predicts a number nobody measured. The mechanism needs none of this: derived rows compete for `top_k` slots they were never meant to occupy, and filtering before the trim returns those slots to real memories. That argument stands without a percentage, and it is what the PR actually established. Same shape as ax-0917-m-28, which we fixed six days ago -- a correctly-sourced figure republished without its conditions until it reads as a general result. Two other mentions were left alone: both already say "one store, one run, one seed". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Arkady Mankovsky <arkash20@gmail.com>
arkash20
added a commit
that referenced
this pull request
Sep 24, 2026
…public repo (#1716) `caura-ai/caura` is **public**. Two places assert a 79.8% → 82.2% (**+2.4pp**) improvement as a fact about `include_derived`, with no provenance and no conditions: - `docs/atomic-fact-fanout/pm-c03-include-derived-blast-radius.md:141` — *"the 79.8% → 82.2% (+2.4pp) result is a ranking gain"* - `tests/test_pm_c03_include_derived.py:6` — *"worth 79.8% vs 82.2% on that harness"*, naming the customer run Both are mine, from #1703 and #1708 yesterday. Filed as `oss-0924-m-02`. ## Why it matters Those figures come from the PersonaMem store — the subject of `pm-0918-c-01` and `pm-0918-c-05`, a store whose headline 85.4% **has not reproduced** across two later runs (82.5%, 82.2%) with only 71% retrieval overlap against its own record. If 85.4% is untrustworthy because of store state, **+2.4pp inherits exactly that doubt**. We were publicly claiming an improvement measured on ground we are simultaneously arguing internally that we cannot stand on. ## Three qualifications, none of which was stated | | | |---|---| | **It is a floor, not an estimate** | Measured **client-side**, before the server-side exclusion that refills `top_k` existed. ~24 rows deleted from 85 and not backfilled — a thinner context than a store that never had them. | | **Recall-boost hysteresis** | `TrackRecalls` bumps `recall_count` on every returned row, children included, and it feeds the score. Query-time filtering cannot undo a counter children already accrued. | | **Wrong run to chain** | A within-run delta on `caura-bulk-2k-top50-sess2`, not the record run. Chaining the two predicts 84.6%, which nobody measured. | ## What replaces it Not a smaller number — **the mechanism**, which needs none of this: derived rows compete for `top_k` slots they were never meant to occupy, and filtering before the trim returns those slots to real memories. That is what the PR actually established, and it stands without a percentage. Two other mentions were deliberately left alone — both already read *"one store, one run, one seed"*. ## Precedent Same shape as `ax-0917-m-28`, fixed six days ago: a correctly-sourced lab figure (23ms p50, warm cache, single-tenant) republished without its conditions in eight places until it read as a customer's production latency. ## Checks Docs and one docstring; no behaviour change. 20/20 tests pass, `ruff check` + `format --check` clean, legacy-name ratchet reports "No new lines". 🤖 Generated with [Claude Code](https://claude.com/claude-code) Signed-off-by: Arkady Mankovsky <arkash20@gmail.com>
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.
pm-0918-c-03 is an API design decision that belongs to Arkady. This PR changes no search behaviour — it adds the evidence needed to take the decision, and a re-runnable measurement so the numbers can be checked against any store.
The finding that makes the decision cheap
Options (a) "request flag
include_derived" and (c) "exclude by default" are the same implementation and one boolean. (c) requires (a)'s flag, because opting back in needs something to opt in with. The decision in front of you is a default value — one line, reversible in both directions at any time — not a choice between three designs.What exactly marks a derived row
Two writers, and only two, set
parent_memory_id, and they are not the same population:source="atomic_fact_fanout"source="auto_chunk"So the predicate must be the conjunction
parent_memory_id IS NOT NULL AND source = 'atomic_fact_fanout':parent_memory_idalone catches auto-chunk children.sourcealone is forgeable —system_metadata.pydeliberately leaves it out ofPLATFORM_ONLY_KEYS(ingest writes it in caller-adjacent metadata).parent_memory_idis reserved, so requiring both halves is unforgeable.source_uri = NULLis incidental, not load-bearing: 19,622 of 55,621 ordinary rows on the dev corpus also have it null.The population — and why the local corpus cannot price this
caura, the only local database written after A70 shipped (2026-09-09), contains zero derived rows of either kind.memclaw's 1,486 children all come from 703 parents that are every onebenchmark=true,write_mode=fast, written on a single day three days before A70.A trap that already cost pm-0918-c-04 one bad table: fan-out children do not inherit the parent's
benchmarkmarker, so a provenance query on the child classifies all 1,486 synthetic rows as real user traffic. The script joins through the parent.The one number that does transfer: within the 195 tenants that have any fan-out, derived rows are 22.4% of the tenant's whole store, which brackets the CEO's ~28%-of-returns from the write side and suggests it is corpus composition, not children out-ranking parents.
Also worth knowing: the existing 2x
SEARCH_OVERFETCH_FACTORalready absorbs a 28% derived rate, so a filter placed inPostFilterResultsbefore the trim fillstop_kexactly — the "ask for 50, get 36" problem does not arise server-side, for any of the three shapes.Contract surface — the question asked directly
POST /api/v1/searchis in the frozen broker subset (gen_broker_openapi.py, called bycloud.Client.Searchin caura-daemon). Flipping a default is a semantic change with no schema movement: oasdiff sees nothing and passes it silently — the ax-0917-m-19 shape. Only theBREAKING CHANGE:trailer andkind/breakinglabel stand in the way, and both are human gates.Recommendation
Build (a) — the flag, default
true— plus a per-tenantsearch.include_deriveddefault, and flip PersonaMem's tenant the same day.search.*is an existing per-tenant boolean namespace, so this is one entry in a shape that already exists, and it gives any store (c)'s behaviour without imposing it on every store. The global default then becomes a one-line change taken later with a second store's evidence.(b) — honour a write-time
metadata.amb_bench— should be rejected outright: it keys read-time policy on a forgeable caller field, it is benchmark-shaped, and A70'senrichment.atomic_fact_fanout_enabledswitch already covers what it was reaching for.There is a real case for (c) and it is stronger than "breaking" sounds: default-on fan-out is 14 days old, so the window for changing it without ceremony is now. If you want (c), take it now rather than later — with the conjunction predicate, the
BREAKING CHANGE:trailer, and a test pinning the default.What would flip this
Full analysis:
docs/atomic-fact-fanout/pm-c03-include-derived-blast-radius.md.Measurement:
python3 benchmark/pm_c03_derived_row_population.py [--db NAME | DSN].🤖 Generated with Claude Code