From 1cc9d96ba6e515a9dd99b1fe5fe2e4aa5e4564e7 Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Fri, 31 Jul 2026 18:12:59 -0400 Subject: [PATCH] =?UTF-8?q?docs(discovery):=20framing=20revision=204=20?= =?UTF-8?q?=E2=80=94=20close=20review=20round=203?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two factual corrections, both accepted and both verified in the code. The custom completion source does not control display order. Revision 3 justified sorting the pool by claiming `Custom` candidates appear in return order; `recompute_candidates` hands the pool straight to `filter_and_sort`, which ranks by fuzzy score descending and breaks ties lexically, so the source's order never reaches the user. The sort is kept, for a reason that is actually true: `filter_and_sort` applies `.take(CANDIDATE_LIMIT)` to the FILTERED iterator before sorting, so when more settings match than the limit, pool order decides which survive truncation. Registration order would make that vary with an unrelated config edit; sorting makes it reproducible. Read-only would not mitigate the foreign-`*help*` collision either. Revision 3 implied it would. A buffer the user created and named `*help*` carries no intercept of ours, so an intercept on the buffers we create protects nothing — the renderer still matches on the name and clears theirs. The missing guarantee is ownership identity: a private table of buffers this module created, so found-by-name is not adoption. `listview` carries it as `panels` and dired as its handle table; this mechanism carries neither. Naming the wrong missing guarantee would send a later fix at the wrong layer, which is why the correction is worth its own paragraph rather than a word swap. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Lv428Fth9LRtffwJSsqH7T --- docs/active-work.md | 23 +++++--- ...discovery-stage1-command-family-framing.md | 52 +++++++++++++++---- 2 files changed, 58 insertions(+), 17 deletions(-) diff --git a/docs/active-work.md b/docs/active-work.md index 298893f..cea9819 100644 --- a/docs/active-work.md +++ b/docs/active-work.md @@ -380,13 +380,14 @@ which would have re-conflicted on every merge. githubsucks/journey-stage1b3-welcome ``` -## Discovery Stage 1 (P4) — FRAMING OPEN, revision 3 +## Discovery Stage 1 (P4) — FRAMING OPEN, revision 4 - **Branch `discovery-stage1-commands`**, worktree `../pmacs-p4-discovery`, based on `githubsucks/main` @ `54a092e`. **Framing only; no code, no - PR yet.** `docs/discovery-stage1-command-family-framing.md` revision 3, - two review rounds closed (round 1: two blocking, two major; round 2: - two blocking, two major; all accepted). + PR yet.** `docs/discovery-stage1-command-family-framing.md` revision 4, + three review rounds closed (round 1: two blocking, two major; round 2: + two blocking, two major; round 3: two factual corrections; all + accepted). - **What it is.** `COHERENCE.md` §20 Priority 4 — "almost pure wiring, the best payoff-per-effort in this document". Nine describe/list commands (describe-key/mode/hook/buffer, where-is, list-commands, @@ -405,7 +406,11 @@ which would have re-conflicted on every merge. Closed-set acceptance ("refuse a non-candidate") is Rust work and is deferred. The custom source needs a **mapper**: `config.list()` yields descriptor *tables* while `Custom` consumes a sequence of - strings. + strings. **It does not control display order** — + `recompute_candidates` runs `filter_and_sort` (fuzzy score, lexical + tiebreak); sorting the pool matters only because `.take( + CANDIDATE_LIMIT)` runs *before* the sort, so pool order decides which + candidates survive truncation. - **`invoke_interactive` is NOT the M-x path** — the error #205 corrected, repeated one PR later. The path is dispatch `M-x` → `editor.execute-command` → assert the selected candidate **before** @@ -419,8 +424,12 @@ which would have re-conflicted on every merge. changes each command's subject-specific logic. What the funnel buys is the shared policy in one place: reuse-by-name, wholesale delete+insert, the `q` binding, and the foreign-`*help*` hazard. - **`*help*` is ordinary editable content** — it has no read-only - intercept and no generated-content invariant. Each command's rendering + **`*help*` is ordinary editable content** — no read-only intercept, no + generated-content invariant. And read-only would **not** fix the + foreign-buffer hazard either: a user's own `*help*` carries no + intercept of ours, so the renderer still finds it by name and clears + it. The missing guarantee is **ownership identity**, which `listview` + and dired both carry and this mechanism does not. Each command's rendering is a named per-subject function so the future Rust work is enumerated per-subject (three new renderers) rather than discovered per-call-site. - **Deliberately deferred, each with a reason:** richer M-x rows diff --git a/docs/discovery-stage1-command-family-framing.md b/docs/discovery-stage1-command-family-framing.md index cc794a5..dd94905 100644 --- a/docs/discovery-stage1-command-family-framing.md +++ b/docs/discovery-stage1-command-family-framing.md @@ -1,11 +1,25 @@ # Discovery Stage 1 — the describe/list command family -**Status: framing, rev 3 — awaiting review round 3.** +**Status: framing, rev 4 — awaiting review round 4.** **Serves `COHERENCE.md` §5 (unify discoverability), §1.1 (substrate without surface), §20 Priority 4.** ## 0. Revision history +- rev 4 (2026-07-31) — review round 3. Two factual corrections, both + accepted. + - **The custom source does not control display order.** Rev 3 + justified `table.sort` by claiming `Custom` candidates appear in + return order; `recompute_candidates` immediately runs + `filter_and_sort`, which ranks by fuzzy score and tie-breaks + lexically. The sort is kept for a reason that is true — `.take( + CANDIDATE_LIMIT)` is applied to the filtered iterator *before* the + sort, so pool order decides which candidates survive truncation. + - **Read-only would not mitigate the foreign-`*help*` collision.** + Rev 3 said it would. A user-created buffer of that name has no + intercept of ours; the renderer finds it by name and clears it + regardless. The missing guarantee is **ownership identity**, the + thing `listview` and dired both have. - rev 3 (2026-07-31) — review round 2. Two blocking, two major; all four accepted. - **The ledger lane still said revision 1 and kept refuted claims.** @@ -256,9 +270,18 @@ source = function() end, ``` -Sorted because `Custom` candidates are presented in the order returned, -and `config.list()`'s order is registration order — which is neither -stable across a config edit nor useful to a reader. +Sorted for **deterministic pool construction**, not for display order — +rev 2's stated reason was false. `recompute_candidates` hands the pool +straight to `filter_and_sort`, which ranks by fuzzy score descending and +breaks ties lexically (`src/minibuffer.rs:672-680`), so what the source +returns never reaches the user in that order. + +The sort still earns its place, for a subtler reason: `filter_and_sort` +applies `.take(CANDIDATE_LIMIT)` to the **filtered** iterator *before* +sorting, so when more than `CANDIDATE_LIMIT` settings match a needle, +**pool order decides which ones survive truncation**. Registration +order would make that selection vary with an unrelated config edit; +sorting makes it reproducible. ### 3.3 `M-x help` becomes the index @@ -280,17 +303,26 @@ shared policies get decided in one place instead of eleven: - **wholesale replacement** — `buf:delete(0, len)` then `buf:insert(0, text)`, never a diff, because `*help*` is reflowed per subject; - the **`q` binding**, rebound per fresh buffer; -- the **foreign-`*help*` hazard** — found-by-name is not ownership, so a - user's buffer of that name is cleared. +- the **foreign-`*help*` hazard** — `find_or_create_help_buffer` matches + on the *name*, so a user's own buffer called `*help*` is adopted and + cleared. **`*help*` is ordinary editable content.** Rev 1 wrote "the read-only intercept"; there isn't one. `show_help_text` writes with plain `delete`/`insert`, the buffer keeps its undo history, and #205 already recorded that this mechanism has **not** adopted the generated-buffer -write invariant. Saying otherwise would have had this stage claim a -guarantee it does not provide — and the fourth policy above is exactly -the hazard that a read-only intercept would have mitigated and does -not. +write invariant. + +**And read-only would not fix the fourth policy either** — rev 2 implied +it would. A buffer the *user* created and named `*help*` carries no +intercept of ours, so an intercept on the buffers we create protects +nothing: the renderer still finds theirs by name and clears it. The +missing protection is **ownership identity** — a private table of +buffers this module created, so found-by-name is not adoption — which +is exactly what `listview` (`panels`) and dired (its handle table) both +carry and this mechanism does not. Naming the right missing guarantee +matters, because the wrong one would send a later fix at the wrong +layer. **What it does not buy, and rev 1 claimed it did:** a one-site migration to `src/help.rs`. That layer has semantic renderers for **command, key,