feat(search): reach the curated layer at --limit 3 — conditional deep fetch + reserved slot (#526) - #534
Merged
Conversation
|
Warning Review limit reachedNext included review available in 27 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (8)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This was referenced Sep 18, 2026
…rved slot) In a wing with ~941K transcript drawers `mempalace search --limit 3` returned three session transcripts and the reader concluded the corpus held nothing curated. The first curated document ranks 14-27 (#526). TWO causes, both measured on production before this was written. 1. The requested limit sizes the hybrid FUSION CANDIDATE POOL, not just the cut. limit=3 and limit=30 return different top-3 drawers for the same query, while the bm25 route is stable across limits; repeated identical calls are deterministic, so this is not drift. At --limit 3 the curated document is not ranked below the cut — it is never a candidate. #477's widen already fired here but computed min(n*2, 40) = SIX at limit 3, and the curated layer begins at 14: it fired and landed short. 2. #477's ordering cannot lift it. That ordering is bounded to near-duplicates by design, and review hardened the bound twice. Measured similarity between the first curated hit and the top-3 transcripts above it: 0.0/0.0/0.0 on one query, 0.008 on another, against a 0.35 threshold. #477 fixes "a transcript that QUOTES the card outranks the card"; this is "the same topic with no shared 3-grams". Different defect, and re-ranking is the wrong instrument. So: a conditional deeper fetch to max(limit, 30) — a floor, not a multiple — fired only when the shallow result has no curated hit; then one reserved slot, the LAST of the N, for the top curated hit in the ranker's own order. The promotion is MARKED on both channels: `promoted: true` and `promoted_from_rank` on the hit in --json, `⟨curated, promoted from rank K⟩` in prose, plus top-level `curated_first_rank` = the rank in the deep fetch BEFORE reordering. Taken after reordering it would be 1 almost every time and say nothing about corpus depth. A silent promotion would be #526's own error in reverse: the reader could not tell "ranked here" from "reserved here". Conditional because the depth is not free: a limit-30 hybrid call on wing 2g measured 19-22 s (#533). Measured after: base 14.49/14.46 s vs head 14.33/15.92 s on the hybrid-fallback query, and 0.18-0.26 vs 0.16-0.24 s on the bm25 route (n=2 per arm, interleaved) — no measurable regression, because the cost is the hybrid call itself rather than the row count. `_WIDEN_FACTOR`/`_WIDEN_CAP` are gone; the three #477 tests that asserted that arithmetic are updated to the new contract rather than deleted. Curated means source_kind file OR memory, matching no_curated_source and prefer_curated — using "file" alone would let a memory-only result trigger the deep fetch while reporting curated_first_rank: null. Part of #526 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
seq from --next-seq taken after the final fetch, commit: HEAD for the merge step to resolve. All three renderers; check-docs clean 7/7. Part of #526 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Part of #526 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The `⟨curated, promoted from rank K⟩` marker lived in `_provenance_tag()`, whose only caller is `_print_hit_compact()`, so the DEFAULT `table` view (and `full`) printed the reserved hit as an ordinary third result — the silent promotion the slot exists to prevent (Oracle PART 44 on #534). One producer, `_promotion_tag(hit)`, now called from `_print_hit_table` (table + full) and from `_provenance_tag` (compact). One test per renderer replaces the single compact-only prose test, plus a ranked-not-promoted negative on table. The two new positive tests fail on 98e25f0 and pass here. Entry: the five acceptance K values are stated as-of-date — wing 2g is re-mined continuously, so the invariants are the durable check. Part of #526 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
jphein
force-pushed
the
feat/526-retrieval-depth
branch
from
September 18, 2026 21:23
98e25f0 to
d749df8
Compare
This was referenced Sep 18, 2026
jphein
added a commit
that referenced
this pull request
Sep 19, 2026
…the clean partition One pre-registered run on main 9db6577: independent-fresh recall@3 0/21, 16 of 21 record files absent from the top 30. B-blind's 6/6 is self-quotation (the wing had mined today's transcripts carrying the trigger↔slug CSV line; rank_of accepts any hit naming the slug, #539) — 2/6 on record files. Points at the two independent fixes (#539 file-only scoring; keep trigger tables out of mined sessions), the retrieval dependency (#526 / PR #534), and the overlap columns' two-way control as the diagnostic for the re-run. Part of #524 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
jphein
added a commit
that referenced
this pull request
Sep 19, 2026
"no curated document matched" is a claim about the STORE. What was measured is the RETURNED SET, and in a wing with ~941K transcript drawers the curated layer begins at rank 14-27, so at --limit 3 the sentence was true of the result and false of the corpus — read by two sessions as "this project has nothing curated on that" (#526). Three cases now, on one result object: curated ranked inside the limit -> nothing said about curation curated promoted from rank K -> "1 curated hit promoted from rank K; the curated layer in this wing begins there - widen the pool with --limit 30" none within the deep fetch -> "no curated document in the top N", and NO rank clause, because no K exists The advice names the mechanism. The requested limit sizes the hybrid fusion candidate pool (#534), so --limit 30 is not "scroll further down the list", it is "widen the pool" — the document was never a candidate. JOINT PRODUCER/CONSUMER TEST, written before the producer shipped so the field's shape was fixed by the consumer's need: tests/ test_search_banner_contract.py drives one result object from curated_first_rank (#534) to the rendered banner across all three cases and asserts the printed K equals the K in --json, parametrised at 14 and 27. A banner whose number disagrees with the field is worse than no number: one reader acts on the prose, another parses the field. The old all-transcript / transcript-plus-diary split is dropped from the banner; the per-hit ⟨transcript⟩ / ⟨diary⟩ tags carry it, and at header level the useful statement is the depth. all_transcript() consequently has no production caller — kept as a tested predicate, with its "for the header line" docstring corrected so it does not describe behaviour that no longer exists. Three #477 banner tests asserted the old wording; updated to the new contract, not deleted, and one now also asserts the phrase "no curated document matched" is ABSENT. Part of #526 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
jphein
added a commit
that referenced
this pull request
Sep 19, 2026
## fix(search): the banner states the DEPTH, not the store (#526, PR 2 of 4) Stacked on #534 (curated_first_rank producer). Part of #526. **Defect.** `! all N hits are session-transcript copies — no curated document matched` is a claim about the STORE; the measurement was of the RETURNED SET. In wing 2g the curated layer begins at rank 14-27, so at `--limit 3` the sentence was true of the result and false of the corpus, and two sessions concluded the project had nothing. **Change.** One banner, three cases, keyed on `curated_first_rank` from #534: | result object | banner | |---|---| | curated ranked inside the limit | (nothing about curation) | | curated promoted from rank K | `! 1 curated hit promoted from rank K; the curated layer in this wing begins there — widen the pool with --limit 30` | | none within the deep fetch (`curated_first_rank: null`) | `! no curated document in the top N` — no rank clause, no K exists | "Widen the pool" is the mechanism (#534: the limit sizes the hybrid fusion candidate pool), not "scroll further". **Producer/consumer pair, one test.** `tests/test_search_banner_contract.py` drives one result object from the field to the rendered line for all three cases and asserts printed K == `--json` K at 14 and 27. Negative: the phrase "no curated document matched" is asserted ABSENT. The banner is only reached when `results` is non-empty, so an error payload cannot print it (PR 4 hardens the error path itself). **Blast radius.** `_print_search_header` (cli.py), `all_transcript` docstring (provenance.py — no production caller now, kept + tested), 3 #477 banner tests updated to the new wording. No JSON field changes. **Verification.** Rebased onto 69fd30c (#534 squash): full suite 7559 passed / 82 skipped / 1 failed — the one failure was `test_quiet_suppresses_the_banner`, whose substring `promoted from rank` now also matches the per-hit marker #534's PART 44 fix added to the table renderer; sharpened to assert the banner SENTENCE is absent under `--quiet` and the per-hit marker survives (quiet strips chrome, not provenance). After: banner contract 8/8 + depth 16/16; ruff check + format clean; check-docs clean (see entry commit). Order note: lands after #536 and #541 in the cli.py queue — expect one generated-only rebase at go. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
jphein
added a commit
that referenced
this pull request
Sep 19, 2026
… both markers #534's promotion tag and this PR's collapse tag were already composed in both renderers on this branch (_print_hit_table's header line; the _provenance_tag return). With #542 on main the composition is a named producer→consumer pair rather than an incidental one: one result object whose reserved hit also absorbed a duplicate, driven through table, full and compact, must show both tags on the SAME line, and --json must carry promoted/promoted_from_rank and duplicates_collapsed/duplicate_of on the same hit. Four tests (3 parametrised + JSON). Entry test count 13 → 17. Part of #526 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
jphein
added a commit
that referenced
this pull request
Sep 19, 2026
…tch and reserved slot (#526) (#543) ## feat(search): collapse identical-text hits before the curated deep fetch and reserved slot (#526, PR 3 of 4) Off `69fd30c1` (#534 squash). Part of #526. **Defect.** A document mined at two paths — `docs/foo.md` and its `docs/rescued-from-vartmp-20260905/foo.md` copy — comes back twice, so `--limit 3` spends two of three slots on the same words. Existing mechanisms checked first: `prefer_curated` reorders near-duplicates but never removes; `searcher._dedupe_rendered_hits` keys on the SAME path and closet hits only. Neither closes this. **Change.** `result_ordering.collapse_identical_text(results)` — in place, idempotent over an already-collapsed list, never raises, same contract as `prefer_curated`. | aspect | rule | |---|---| | identity | `text` with whitespace runs collapsed; nothing looser — a one-line edit stays two documents | | position | first occurrence keeps the ranker's slot | | payload | a later higher-trust copy (memory < file < transcript < diary via `_kind_rank`) takes the slot and inherits the bookkeeping — same words, the copy a reader can cite | | JSON channel | kept hit gets `duplicates_collapsed: N`, `duplicate_of: [paths…]` — per hit, never summed | | prose channel | `⟨N identical copies collapsed⟩` from one producer `_collapse_tag`, rendered by `table`/`full` (`_print_hit_table`) and `compact` (`_provenance_tag`) — one test per renderer | | where | fast, hybrid and MCP-fallback routes, after `annotate`, BEFORE `_deep_fetch_when_nothing_curated`; and on the deeper list | | depth trigger | also fires when a collapse left the shallow list shorter than the limit — one extra call, only when a duplicate was seen; `call_count == 1` on the common path (tested) | **Producer/consumer, one test.** `test_collapse_runs_before_the_curated_deep_fetch_and_reservation`: the curated copy at rank 14 says the same words as the transcript at rank 2; collapsed first, the curated copy takes rank 2 on its own merit — visible at `--limit 3` with NO `promoted` flag and `curated_first_rank == 2`. That is the "before reservation" claim, executed. **Positive control on the base 69fd30c.** 11/13 new tests FAIL there (5 helper tests by ImportError, T1/T4 and all three renderer tests by assertion); 2 pass and are marked in their docstrings as regression guards, not discriminators (`no_second_call_when_nothing_collapsed…`, `table_does_not_mark_unique_hits`). **Existing tests changed (contract, disclosed).** Three fixtures in two existing test files had two or more hits with identical text — exactly what this collapses — and were given distinct texts so each keeps testing what it names: `test_cli_daemon::test_widen_tolerates_a_failed_second_call` (two `UNRELATED` transcripts → 1), `test_cli_daemon::test_widen_that_finds_nothing_curated_still_reports_honestly` (4 identical → 1, silently weakening it), and `test_cli_search_output::test_one_line_per_hit` (two default `_hit()`s). **Composed marker (PART 50 go-time condition).** #534's `_promotion_tag` and this PR's `_collapse_tag` are composed in BOTH renderers — `_print_hit_table`'s `[N] wing / room` line (table + full) and `_provenance_tag`'s return (compact) — so a hit that is both reserved and a collapse anchor shows both. Named producer→consumer pair: `test_a_hit_that_is_both_promoted_and_a_collapse_anchor_shows_both` (parametrised table/full/compact, asserts both tags on the SAME line) and `test_json_carries_both_markers_on_the_same_hit`. **A fixture trap worth naming.** My first shallow/deep payloads shared dict objects, so collapsing the deeper list re-counted the kept dict (`duplicates_collapsed: 2`). The daemon returns fresh JSON per call; the fixtures now `deepcopy`, with a comment saying why. **Blast radius.** `result_ordering.py` (+1 public helper, `__all__`), `cli.py` (`_collapse_tag`, `_provenance_tag`, `_print_hit_table`, `_deep_fetch_when_nothing_curated` gains `short=`, `_daemon_search_fast`, `_daemon_search_hybrid`, MCP fallback in `cmd_search`), 2 existing test fixtures, 1 new test file (13 tests). **Verification.** Full suite 7565 passed / 82 skipped / 0 failed (rc from the same invocation); targeted 230/230 across 7 search/render files; `ruff check` + `ruff format --check` clean; check-docs (see entry commit). Landing order: after #542 in the cli.py queue (`_provenance_tag` is touched by both — expect one textual rebase at go). 🤖 Generated with [Claude Code](https://claude.com/claude-code)
jphein
added a commit
that referenced
this pull request
Sep 19, 2026
…its (#526) (#545) ## fix(search): a daemon error object is an ERROR with a code, never 0 hits (#526, PR 4 of 4) Consumer of #536's option-C error shape (`{error, code, source, status?, detail?}` — lucid-error-contract owns the shape; this PR adds one key to the documented set and emits it). **Part of #526** — deliberately not "Closes": #526 enumerates three fixes (banner → #542, retrieval → #534, dedup → #543), all merged before this PR; this fourth PR is a defect found while measuring those three (a busy daemon rendering as an empty corpus), adjacent to the umbrella rather than one of its parts. #526 can be closed by hand once this lands, citing the four. **Defect, measured on production.** A saturated daemon answered `/search/hybrid` with HTTP 200 and `{"error": {"code": -32003, "message": "daemon busy: 8 MCP tool call(s) in flight (PALACE_MCP_TOOL_MAX_INFLIGHT=8)"}}`. The REST transports returned it verbatim, the search helpers read `.get("results") or []`, and the CLI printed **0 hits, exit 1**. A busy daemon was indistinguishable from an empty corpus — and the depth banner would then have said no curated document was in the top N. That is #526's own error class inside #526's fix. **The base defect is wider than search, and one grade worse in one verb.** On base 8408188, `status --json` against a 200 + `-32003` error object **exited 0**, echoing the error object as if it were the payload — a SUCCESS exit carrying an error (Oracle PART 51). On this head every daemon-backed verb that reaches a transport — 7/7 driven by Oracle with a busy body — exits 2 with `code: "daemon_busy"`, because the classification lives in the three transports rather than in any one verb. **Change.** | piece | rule | |---|---| | classifier `_raise_if_daemon_error_object(body, route)` | keyed on the PAYLOAD: dict with `error` and neither `results` nor `result`. `-32003` or "busy" in the message → `DaemonBusyError`; anything else → `DaemonError("daemon error <code> on <route>: <msg>")` (the prefix `_fail_daemon` keys its reachable line on). A body with `results` beside an `error` passes through. | | producers | all three transports call it — `_call_daemon_rest`, `_post_daemon_rest`, `_call_daemon_tool` — so REST and MCP agree. In `_call_daemon_tool` the classifier raises for every error envelope, so the pre-existing `raise DaemonError("daemon error <code>: <msg>")` below it was dead and is deleted; the message now carries the route (`… on /mcp <tool>: …`), which the old one lacked — kept on purpose. Tests that build that string by hand as a side effect are unaffected. | | renderer | `cmd_search`'s `except DaemonError` → `_fail_daemon(e, want_json, route=…, query=…)` (the one search-shaped site #536 did not reach). New busy branch emits a **dict literal**: `{"error": prose, "code": "daemon_busy", "source": "daemon", "detail": <daemon words>, "route": …}`, exit 2; prose: `palace daemon at <url> is busy — <words>; retry shortly` on stderr | | documented set | `daemon_busy` appended to the header line AND to `BRANCHABLE_CODES` in `test_cli_daemon_error_contract.py` — both or neither | | optimisations degrade, never silently | `_deep_fetch_when_nothing_curated(..., warnings=)` appends `deeper fetch unavailable: <daemon words>`; auto-mode's swallowed hybrid fallback appends `hybrid fallback unavailable: …`. The header already prints `warnings`, so `curated_first_rank: null` is read as "the depth was not checked" | **Why a literal.** `test_every_code_we_emit_is_in_the_documented_set` walks dict literals only (emitted ⊆ documented). Adding `daemon_busy` to the header without a literal emitter would be unverified vocabulary reading as a contract. `test_daemon_busy_is_emitted_as_a_dict_literal` is the converse for this one key — safe here because the emitter is known to be a literal; the general converse would false-positive on `_fail_daemon`'s computed `code`. **Pre-registration amendment, stated.** I pre-registered "2nd response busy → exit 2, never `curated_first_rank: null`". That contradicts the tested contract that the deeper fetch is an optimisation (`test_widen_tolerates_a_failed_second_call` degrades to the shallow hits). Replacement: degrade AND carry the daemon's words in `warnings`; joint producer→consumer test (`test_busy_deeper_fetch_keeps_the_shallow_hits_and_warns` + `test_the_header_prints_that_warning`). **Positive control on the base (#536 @9441576b, unfixed).** 15/15 substantive tests FAIL there (4 exit/code, 3 degrade, 3 documented/literal, 4 classifier, 1 MCP). A filler test written to justify an import was deleted rather than kept. **Blast radius.** `cli.py`: `DaemonBusyError` (new class), `_raise_if_daemon_error_object` (new), `_call_daemon_tool`, `_call_daemon_rest`, `_post_daemon_rest`, `_fail_daemon` (busy branch), `cmd_search` except, `_deep_fetch_when_nothing_curated` (`warnings=` kwarg), `_daemon_search_fast` (adds `warnings` only when non-empty — return shape otherwise unchanged), `_daemon_search_hybrid`, `_daemon_search_auto`; header contract line; `BRANCHABLE_CODES`. `_window_daemon_get` is deliberately untouched — different producer, same payload rule would apply if it ever returned a 200 error object. **Verification.** Stacked on #536 @9441576b: full suite 7596 passed / 82 skipped / 0 failed (rc from the same invocation); 330/330 across 12 files including #536's own contract/4xx/one-message/exit-propagation/window-source/cypher/stats suites; `ruff check` + `ruff format --check` clean; check-docs (see entry commit). Lands after #536; expect one generated-only rebase at go. 🤖 Generated with [Claude Code](https://claude.com/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.
What
mempalace search --limit 3can now return a curated document in a wing where thecurated layer begins at rank 14–27. Two mechanisms, a conditional deeper fetch and one
reserved, marked slot. #526 fix 2.
Why — two causes, both measured on production before this was written
1. The requested limit sizes the hybrid fusion candidate pool, not just the cut.
So at
--limit 3the curated document is not ranked below the cut — it is never acandidate. #477's widen already fired here, but computed
min(n*2, 40)= 6 atlimit 3 while the curated layer begins at 14: it fired and landed short.
2. #477's ordering cannot lift it, and that is by design.
#477 fixes a transcript that quotes the card outranks the card — near-duplicate
ordering is exactly right for that. #526 is the same topic with no shared 3-grams.
Two review rounds hardened #477's bound deliberately; it is not under-tuned, it is the
wrong instrument. Relaxing it would mean dropping the threshold to ~0, i.e. "curated
always first". Hence a reserved slot rather than a re-rank.
How
max(limit, 30)— a floor, not a multiple — fired onlywhen the shallow result contains no curated hit.
_WIDEN_FACTOR/_WIDEN_CAPare gone.order, only when the top N would otherwise contain none.
reverse — the reader could not tell "ranked here" from "reserved here":
--json:promoted: trueandpromoted_from_rankon the hit, plus top-levelcurated_first_rank⟨curated, promoted from rank K⟩curated_first_rankis the rank in the deep fetch before reordering. Taken after,it would be 1 almost every time a curated hit exists and would say nothing about
corpus depth — which is the one thing the banner in PR 2 needs it to say.
Producer/consumer: the daemon's
/search/fastand/search/hybridpayload (the deepfetch) produces; the CLI's truncation consumes, after
#477'sprefer_curated. Both aredriven together in
tests/test_search_depth.py.curated_first_rankis itself theproducer for PR 2's banner, and the joint test that pins their agreement is already
written and lands there.
Acceptance — production, wing 2g,
--limit 3, read-onlyFive queries pre-registered before any code. Q1–Q4 are the published defect queries; Q5
is a positive control (lead-approved substitution — 2g-c6's query string was never
recorded anywhere), present so a table that reads all-zero cannot be mistaken for a
working instrument.
curated_first_ranknullFour of five now return a curated hit where the base returned zero. Q2 is reported
honestly: on the bm25 route it takes, no curated document lies within depth 30, so it
returns
nullrather than inventing a rank. Its curated hits are reachable on thehybrid route (ranks 14/15/24/25/30) — a route-selection limit, not a depth one, filed separately as #537 rather than widening this PR.
Latency, both arms, interleaved, n=2 per arm
No measurable regression: the cost is the hybrid call, not the row count — which is
also why the trigger stays conditional rather than deepening every search (#533).
Tests
15 new in
tests/test_search_depth.py: the depth floor (driven through the helper,because a route-level test cannot see it once the shallow fetch already returned
limitrows), the conditional trigger and its no-op cases,curated_first_rankpre-reorder and
null, reservation of the last slot, both marking channels,--limit 1, and "no promotion claimed when it ranked there".Three #477 tests asserted the
min(n*2, 40)arithmetic this PR removes; they areupdated to the new contract, not deleted, and say why in their docstrings.
Full suite 7517 passed, 0 failed.
ruff checkandruff format --checkclean.scripts/check-docs.shclean 7/7.Part of #526