commit 75688a48d31df70891cbab365aa2461ff3187c5b
parent e2bbf04267992ce974da034149f64415a02d6b65
Author: I Mean I'm Just Saying <imeanimjustsaying@kiwifarms.st>
Date: Sat, 12 Sep 2026 11:58:23 -0400
plans: the S3 note says what the slice did, not what it meant to do
Review findings F1, F2 and F3 corrected in §Record, each as a marked
block under the divergence it belongs to rather than a silent edit — a
note that quietly rewrites itself is worth less than one that shows where
it was wrong.
F2 is the one with no code behind it: the note claimed "every name
components/searchPipeline exported it still exports". Three were removed —
createSearchPipeline, createPostsSearchPipeline and
createSubsSearchPipeline. No package imports any of them (they were only
ever reached through runLeafPipeline, which dispatches on leaf scope), so
they moved down and stayed internal to lib/search/leafPipeline.ts.
Re-exporting three functions nobody calls to keep a sentence true would
have been the wrong trade.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Diffstat:
1 file changed, 44 insertions(+), 5 deletions(-)
diff --git a/plans/one-core-phase-2.md b/plans/one-core-phase-2.md
@@ -386,7 +386,8 @@ is S3's gate, after the search pipeline lands.
### S3 — shipped 2026-09-12
Branch `one-core/phase-2-s3`, off `7f86aef` (the `integrate/2026-09-storage-priority`
-tip with S1 merged). Six commits, `eaa50b6` → `99fe37a`, plus this note, unmerged.
+tip with S1 merged). Six commits, `eaa50b6` → `99fe37a`, plus this note; then three review
+fixes, `172099c` → the commit that corrects this note. Unmerged.
**No URL shape moved, no `corpus.json` byte moved, no CONTRACT version moved, no
architecture allow-list entry added — and `"components"` is now in
`FORBIDDEN.lib`, so the list of forbidden edges grew while the list of excused
@@ -399,7 +400,10 @@ ones did not.**
| `07075d6` | the inversion: four `lib → components` edges deleted, `"components"` added to `FORBIDDEN.lib` |
| `fb6a31d` | the unused `LeafMatcher` import the previous commit left |
| `99fe37a` | the `SearchMode` doc comment travels with the type |
-| _(this commit)_ | this note |
+| `726de3e` | this note, as first written |
+| `172099c` | **F3** — `lib/search` holds no caller's budget as a default |
+| `752ceb1` | **F1** — the result list calls `rankByUploadDateDesc` |
+| _(this commit)_ | **F2** + the F1/F3 corrections to this note |
#### What the slice actually found, where the brief and the code disagreed
@@ -427,6 +431,15 @@ comparators in `rank.ts` and both call sites call them. **No relevance
ranking was invented** — that would have changed what every caller returns
under cover of a refactor.
+> **Corrected after review (F1).** As first written this note was wrong
+> about its own slice: `getThread` was converted but
+> `SearchSessionContext.tsx:886` still sorted inline, so `rank.ts` shipped
+> with the viewer's ordering in it and zero viewer callers — a second copy
+> with better documentation, not a shared comparator. Fixed in `752ceb1`;
+> the helper IS the inline ternary, sorts in place and returns the same
+> array, and `Array#sort` is stable, so same-date rows keep the order the
+> two append loops built.
+
**3. `collapse.ts` has one consumer, and the viewer's "duplicate-collapse"
is not one.** `SearchResults.tsx:502` is `duplicateSiblings(...)` from
`components/duplicatesCache.ts` — it decorates a card with a sibling
@@ -461,9 +474,22 @@ Re-grepping `lib/` found four, and all four had to go before
`runQueryTree` gains one required `runtime` field. Its three callers pass
`searchRuntime`: `SearchSessionContext.tsx`, `charts/useSearchSeries.ts`
-and `export/app/lib/askRetrieval.ts`. No other import site moved — every
-name `components/searchPipeline` exported it still exports, including the
-`LayerHit` that `askRetrieval.ts` imports from it.
+and `export/app/lib/askRetrieval.ts`. No other import site moved: every
+name `components/searchPipeline` exported **that anything imported** it
+still exports, including the `LayerHit` that `askRetrieval.ts` imports
+from it.
+
+> **Corrected after review (F2).** The first wording said "every name it
+> exported", which is not true and worth being exact about: three exports
+> were REMOVED — `createSearchPipeline`, `createPostsSearchPipeline` and
+> `createSubsSearchPipeline` (`7f86aef:searchPipeline.ts:54,300,454`).
+> Grepping every package found no importer of any of them; they were only
+> ever reached through `runLeafPipeline`, which dispatches on the leaf's
+> scope. So they moved down to `lib/search/leafPipeline.ts` and stayed
+> module-internal to it rather than being re-exported from a binding whose
+> whole point is to answer one question. Re-exporting three functions
+> nobody calls would have been publishing a surface to preserve a
+> sentence.
**6. `window.ts` keeps TWO excerpt shapes on purpose.** The stub hoped
"the viewer's excerpt and the MCP's excerpt are the same excerpt". They
@@ -481,6 +507,19 @@ that says what uncapped MEANS, and a test pins the load-bearing
consequence (`truncate(t, Infinity)` is the identity;
`0 >= Infinity` is false).
+> **Hardened after review (F3).** As first written, `truncate`'s `max` and
+> `RecordCtx.snippetChars` defaulted to `MCP_POLICY.snippetChars`, so
+> `lib/search/` held one caller's budget as the shared default — and the
+> failure mode was the bad kind: a viewer adopter that forgot to pass its
+> policy would not break, it would silently clip every excerpt at 240 with
+> the whole suite green. All three widths (`truncate`'s `max`,
+> `windowedTranscript`'s `maxLines`, `RecordCtx.snippetChars`) are now
+> REQUIRED, `MCP_POLICY` is imported nowhere under `lib/search/` except by
+> the module that defines it, and two tests run the same input under both
+> policies and assert the widths differ. Fixed in `172099c`; mcp already
+> passed its policy explicitly at every site, so **no read path changed and
+> the bench counters cannot move**.
+
**8. `buildScanPlan` stayed in `mcp/src/search.ts`.** It is reader-driven
orchestration, `scanPlan.test.ts` sits beside it, and it calls the single
`passesFilters` from `lib/search/evalTree` — so the pruner and the scanner