COHERENCE.md §1.2 makes "a `pcall` around background wiring must log
attributed failure, never discard it" a standing rule, and names
`ensure_server`'s swallowed spawn failure as its canonical case — the
exact function this branch modifies. Round 1 deferred the resolver's
silent `pcall` as a Stage 3 concern. Under that rule it is not a
deferral, it is a fresh instance of the named anti-pattern added by a PR
touching the cited function, made worse by the memo: a raised error is
buried permanently for that directory and never observed again.
A resolver that raises, or returns a non-string non-nil, now leaves an
attributed trace naming the language and the directory. Returning nil
remains the documented decline and stays silent — pinned, so "report
failures" cannot be satisfied by reporting every resolution.
The report goes through `pmacs.editor.set_status`, NOT `pmacs.error`,
and that choice is the finding:
**`pmacs.error` does not exist.** Fifteen call sites across `async.lua`
(5), `syntax.lua` (4), `lsp.lua`, `mcp.lua`, `fs.lua`, `editops.lua`,
`autosave.lua`, and `commands/default.lua` report background failures
through it, each guarded `if pmacs.error then ...`. It is defined
nowhere in production; the only assignment in the tree is a test stub at
`src/editor.rs:9881`, and `type(pmacs.error)` is nil in a fresh
`EditorState` (probed, not inferred). `pmacs.errors` (plural) in
compile.lua is an unrelated namespace. So all fifteen reports are dead,
and the guard makes the silence look deliberate — which is why nobody
noticed. Writing the test is what caught it: the first version of this
fix used `pmacs.error` and its pin failed against a working
implementation.
Both bites recorded: dropping the report entirely fails the pin, and so
does reporting ONLY through `pmacs.error` — the dead-channel variant
this nearly shipped.
Not fixed here, deliberately: defining `pmacs.error`, the fifteen dead
sites, and surfacing the spawn failure itself. That last is Priority 1
work and a user-visible product behavior — what message, where, with
what guidance — so it needs its own framing rather than being smuggled
into an affinity PR.
Lands the approved dired framing on main as its own docs PR, and brings
the two required docs current after find-file merged as #162.
The framing was approved after two review rounds (seven findings, then
six) and revised twice more since: revision 4 recorded what implementing
Stage 0 falsified in the approved text, and revision 5 adds the coherence
impact statement that #163 made mandatory for every framing.
The coherence statement is new work, not a restatement. COHERENCE.md
section 20 Priority 1 already names this arc -- a find-file surface and
directory-argument handling -- so the framing now states which journey
steps it touches (7, and partially 3), that it adds no interaction island
because its keys are a mode-scoped keymap through the ordinary registry
and wdired is a mode swap rather than a modal layer, that it adopts the
config registry for dired.kill-when-opening, and that it inherits the
worker-attribution gap for its read_dir jobs without worsening it. It
also draws the boundary against the adjacent Journey Stage 1 arc: CLI
directory handling belongs there, the two meet at resolve_target_buffer,
and dired supplies the buffer a directory should resolve to rather than
growing a second directory surface.
One convergence worth recording: section 2 grades the golden journey
broken at step 3 because pmacs on a directory exits 1, and the mechanism
it cites -- File::open succeeding on a directory, then read_to_end
returning EISDIR -- is the same one Stage 0 pinned in its
accepting-a-directory test, where the pcall turns it into a status
message instead.
The handoff snapshot was stale through eight merges. It now anchors on
main at 2af1ab3, records COHERENCE.md as required reading and a required
framing input, and carries the two minibuffer facts find-file
established: a custom completion source cannot descend directories, and
a selected candidate shadows typed text -- both of which apply to M-x and
switch-buffer, not just find-file.
The ledger gains the dired lane with Stage 1's scope, the reason its one
Rust change cannot be done in Lua, and the rebase note for the dired
branch, whose framing commits become redundant when this lands.
COHERENCE.md states the product-coherence thesis (pmacs should be
immediately excellent, progressively understandable, completely
inspectable, and ultimately replaceable) and, per-section, the audited
ground truth of how the codebase measures against it: a scorecard
across 19 concerns, the golden-journey verdict table (breaks at "open a
real project" -- `pmacs .` exits 1), the six hardcoded key-interception
shadows with no transient-keymap mechanism to migrate them to, the
discoverability substrate-without-surface gap, the package/worker
identity gap, and three cross-cutting patterns (substrate without
surface, the silence asymmetry, per-arc coherence debt) that explain
most of the individual findings.
CLAUDE.md and AGENTS.md now list it as required reading alongside
agent-handoff.md and active-work.md, and ask new framing docs to state
their coherence impact. No runtime code changes.
Three of the five review findings land here; the other two are recorded
as named deferrals in the framing on the dired branch.
Finding 1: the command comment cited "acc4", a name from a draft scheme
that no test carries. It now names the real test, and the comment splits
the shadowing consequence into the two cases that actually exist -- a new
bare name that matches an entry (shadowed) versus one that matches
nothing (creates normally) -- each pointing at its test.
Finding 2: the everyday new-file flow had no test. Typing a bare name
that is not a subsequence of any entry is the path users hit first, and
the only route combining free text with a relative join; every existing
new-file test used a name containing a separator.
find_file_bare_new_name_creates_in_the_root covers it, asserting the
parent is the prompt's root so the join itself is pinned.
Finding 3: the failure arm was never exercised, and as the review noted,
deleting the pcall would have passed the whole suite. Accepting a
directory candidate reaches display_file, whose load fails because
File::open on a directory succeeds and the read returns EISDIR;
find_file_accepting_a_directory_reports_instead_of_raising pins that this
surfaces as the command's status message, leaves the active buffer alone,
and closes the prompt. Verified by manual revert: with the pcall replaced
by a direct call, that test and only that test fails. scripts/bite could
not isolate it, since the guard and its test have no separating commit.
Finding 4 is documented at the command rather than left implicit:
accepting on empty input opens the first-sorted candidate, because
fuzzy_score returns Some(0) for an empty needle and filter_and_sort
breaks the tie lexicographically, so dotfiles lead and a directory can
lead. M-x and switch-buffer share the mechanism, so it is inherited
rather than introduced, and it is listed in the framing beside the
accept-semantics change that would close it.
The blocker was process, not design. The test file was committed before
`cargo fmt` ran, so the reflow of five over-width assertions sat
uncommitted in the working tree while the branch as pushed failed the
first gate in CLAUDE.md. The "fmt clean" reported on the PR described
the worktree, not the branch. Gate results are only meaningful run
against the pushed tree, so this commit lands the formatting first and
the gates are re-run against it.
Two pins review asked for, each covering a branch the nine acceptance
tests left untested:
- A **string** `config.root` as an affinity key. acc17 covers only the
function form, so `return configured, "config"` had no test. The bite
puts both files in their own marked project: drop the config arm and
they key on their own detected roots and spawn two servers, so one
server on the configured root is only reachable if the override wins.
- `root = false` reads as unset. Defended by a truthiness check rather
than `~= nil`, previously by comment alone. Under `~= nil` the config
arm returns `false, "config"` and `file_uri_for(false)` returns nil, so
the file lands on a rootless server instead of its detected project.
Each was falsified against exactly the mutation it targets and neither
against the other.
Also documents an asymmetry review caught: `project_root_for`'s
"detected" arm is canonicalized for free because `pmacs.project.detect`
canonicalizes before walking, but a **configured** root — string or
resolver return — is fed to `file_uri_for` exactly as written, and the
affinity key is that URI. On macOS a resolver returning `/var/…` and a
detected `/private/var/…` are therefore different keys for one
directory, silently yielding two servers for one project. There is no
Lua-side canonicalizer to normalize it, and Stage 3's Lean resolver is
the first real consumer, so the obligation is stated in the
`config.root` doc comment where that resolver's author will read it.
Stage 1 merged as #160 (`main` @ `0827dd1`); the Lean lane header and
branch line now say so, and Stage 2 gets its own subsection.
Edits stay inside the Lean lane. PR #156 is still open against both this
file and `docs/agent-handoff.md`, and it rewrites the snapshot header,
the canonical-base line, and the whole bottom-panel lane — so those are
left alone rather than merged twice. `agent-handoff.md` is untouched for
the same reason plus its own: §1 describes what is on `main`, so it
updates at merge, not during review.
Records the one finding this stage turned up but did not fix:
`ensure_server` never forwards `cfg.restart` to `pmacs.lsp.spawn`, so a
`restart` in `pmacs.lsp.config[lang]` is silently dropped on the
auto-attach path. Pre-existing, and out of scope for a PR whose
acceptance 16 pins existing attach behavior as unchanged.
Dired arc Stage 0 (docs/dired-framing.md section 10, Q#DR11). Until now
pmacs had no discoverable way to open a file by path: no find-file
command and no C-x C-f binding, so a file entered a session only from
the CLI, an LSP jump, a project-search visit, or C-x C-r, whose prompt
does pass free text through but completes only over the recent list.
The command prompts with completion rooted at the active buffer's
directory, or the process cwd when the buffer has no backing path, and
opens the result through pmacs.window.display_file. A path that does not
exist yet creates a buffer bound to it with the "[new file]" status,
which is Emacs parity and comes from resolve_target_buffer rather than
anything added here. Nothing is written to disk until the user saves.
Two substrate facts shape the design and are documented at the command
rather than left to be rediscovered.
Completion is flat: the files source lists one directory and yields bare
basenames, and a custom function source could not do better, because
sources are called with no arguments and run synchronously outside any
coroutine, so a callback can neither see the input to re-root on nor
await a directory listing. Hierarchical completion is a named Rust
change in the framing.
A selected candidate shadows typed text: recompute_candidates selects
index 0 whenever the candidate list is non-empty, and
resolve_accepted_value returns the candidate over the typed contents. So
typed text reaches the accept handler exactly when the input filters
every candidate away, which for basename candidates under a subsequence
filter means when it contains a separator. That makes the deeper-path
case work verbatim and leaves one hole: a new bare name that is a
subsequence of an existing entry opens the existing file. The acceptance
pins that as a decision rather than an accident; closing it needs a Rust
change to accept semantics that Stage 0 deliberately does not make.
A leading tilde is expanded before the path reaches the core, because
get_or_load_buffer normalizes the path it stores but loads from the raw
one -- so an unexpanded tilde path deduplicates against an already-open
buffer yet fails to load a file that is not open yet.
The prompt field starts empty and names its root in the prompt string
instead: any prefill would contain a separator and silently disable
completion.
Acceptance is dispatch-driven throughout -- a real C-x C-f, real typing,
a real RET -- so a dead binding cannot pass vacuously and the Lua
lifecycle accept(), which bypasses the path interactive input takes, is
not used.
`ensure_server` reused any live server whose `language_id` matched,
regardless of project root — its own comment documented this as a known
post-v0.1 limitation. For project-model-strict servers that is a
correctness failure, not a rough edge: `lake serve` is bound to one Lake
package, rust-analyzer and gopls to one workspace, so the second project
a user opens gets a server that cannot resolve its imports.
Server affinity is now keyed on the project root, with one rule that
keeps the change from regressing every other language:
The affinity key is the root only when a root was actually FOUND.
`project_root_for` never returns nil for a file that has a path — its
last resort is the file's own directory — so a naive `(language_id,
root)` key would give every directory of loose scratch files its own
server, for every language: two stray .py files in different directories
would spawn two pyrights where today they share one. It now returns
`root, source` with source one of "config" / "detected" / "fallback",
and only the first two become an affinity key.
Matching is on the spawned spec's `root_uri`, nil matching nil, so the
fallback spawn passes `root_uri = nil` for the key and the stored spec to
agree. `cwd` still carries the directory, and `build_initialize`
(src/lsp.rs) derives the identical `rootUri` from `cwd` when the field is
None — using a percent-encoder with the same allowed set as Lua's
`file_uri_for`. The initialize payload for that case is therefore
byte-identical to before; only what the reuse loop matches on changes.
`build_initialize` is the only reader of `spec.root_uri` in the tree.
Two consequences, both deliberate and both asserted rather than
discovered:
- A server hand-spawned from init.lua with only `cwd` set also reads
back nil, so a root-bearing attach will not adopt it. We cannot know
which root it was meant to serve, and guessing wrongly routes a
project's files to the wrong server.
- Opening files across N project roots spawns N servers. rust-analyzer
has the same property and no editor caps it by default; `pmacs.lsp.stop`
is the manual escape and an LRU reaping policy stays deferred.
`config[language].root` may now be a `function(path) -> string|nil` as
well as a string, for languages whose root rule the shared marker walk
cannot express — an innermost-wins walk cannot find an *outermost*
marker. A resolver returning nil declines and falls through to the marker
walk. Results are memoized per directory because hoisting the root
computation above the reuse loop puts it on every attach rather than
every spawn; the memo is keyed weakly by the resolver function itself, so
replacing `config[lang].root` cannot serve a root the old one computed.
`pmacs.lsp.list()` rows gain `root_uri` and `cwd`. `root_uri` is the spec
field verbatim, deliberately not the URI the server was initialized with.
No protocol change. No Lean content: this is the shared affinity function
for every LSP language, so it ships as its own PR and is exercised
through rust, python, go and typescript against `pmacs_fake_lsp`.
tests/lsp_multi_root_acceptance.rs covers acceptance 13-21. Every fixture
sets `pmacs.project.set_search_boundary` at its own tempdir root:
without it the marker walk climbs to the filesystem root, and a stray
`.git` above the temp directory would turn the markerless cases into
detected ones — the assertions would still pass while testing nothing.
Refs docs/lean4-mode-framing.md Q#LN15, acceptance 13-21.
Review round 1 flagged that neither ledger knew about this branch, and
`docs/active-work.md`'s stated job is exactly the volatile open lanes.
Records the branch, base, framing revision, what Stage 1 ships, the
discharged Q#LN1 obligation, the Q#LN4 blast radius, and the four
implementation findings that are not in the framing (the `warning`
colour collision with `number`, `Some(1)` resolving to `@function`
rather than `@constructor`, the `module > declaration > def` nesting,
and `injection_aliases` being a write-only proxy). Also carries forward
the two Stage 2 corrections the framing already holds, since that lane
starts next.
Deliberately ADDITIVE ONLY -- one new section, zero deleted lines. PR
#156 is open against both this file and `docs/agent-handoff.md` and owns
the snapshot header, the canonical-base line, and the bottom-panel
lane's status. Touching those here would collide with a PR already in
review, which is the "frozen reviewed PRs do not absorb moving
overlapping work" lesson from #135/#137.
`docs/agent-handoff.md` is deliberately untouched: its §1 snapshot
describes what is ON `main`, so it gets updated when this merges, not
while it is in review.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review finding: acc12's server-list assertion could not fail for the
regression class it was written to catch. The shared `editor()` helper
runs `pmacs.lsp.config = {}` before any buffer opens, so
`#pmacs.lsp.list() == 0` holds for every language regardless of what
Stage 1 ships -- a Stage-3 front-run that added
`pmacs.lsp.config.lean4` in a builtin runtime file would have slipped
straight past it. The same vacuous-assertion shape as #155 R2.
acc12 now asserts the actual claim against a PRISTINE `EditorState`,
before any config wipe: no builtin runtime file defines
`pmacs.lsp.config.lean4`. A non-vacuity check pins that the same lookup
finds `pmacs.lsp.config.rust`, so this cannot pass merely because the
table is empty or absent.
Bite-verified: adding `pmacs.lsp.config.lean4 = ... { command = "lake",
args = { "serve" } }` to `builtin/runtime/lsp.lua` fails the test; the
stub was reverted.
The process-list half is kept and its comment now says why it survives
the wipe: a direct probe spawn from a future `lean.lua` shows up there
whatever `pmacs.lsp.config` contains.
Also fixes a stale column in a `highlight.rs` comment -- the Lua table
brace in `local t = {}` is at col 10, which is what the code already
used.
Gates rerun: fmt and strict workspace clippy clean; 1,826 default +
2,003 CRDT library tests; lean4 Stage 1 9/9; M4 121; required GPU 152;
isolated-config workspace sweep 3,150 across 90 suites; diff check clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Completes Arc 8 Stage 1: the Lua-side tables that turn a recognized
grammar into a usable mode, plus the acceptance suite for all twelve
framing criteria.
comment.lua -- `lean4 = "--"` (Q#LN5). Line comments only; Lean's block
comment `/- -/` and docstring `/-- -/` belong to the comment arc's own
named deferral and this lane does not front-run it.
pair.lua -- `⟨⟩`, `⦃⦄`, `⟮⟯` alongside the ASCII brackets (Q#LN6). The
anonymous constructor is among the most-typed constructs in Lean;
omitting it would make the pair set feel broken. The other two ride along
because the Stage 4 input method can produce them, and a bracket the pair
set does not understand is worse than one it does. All three sit outside
the nine built-in pair chars, so per Q#AP1 their undo is
cross-peer-degraded -- the documented, pre-existing limitation of
user-extended pairs. No `''`: Lean uses the prime as an identifier suffix
(`h'`, `foo'`), the same reason Rust excludes it.
syntax.lua -- the `lean` -> `lean4` modeline alias (Q#LN2), so an Emacs
`-*- mode: lean -*-` or a Vim `ft=lean` line is not stranded by the entry
being named `lean4`.
syntax.rs -- the `lean` -> `lean4` injection alias (Q#LN17), so both
```lean and ```lean4 fences highlight. The Lean 3 spelling is mapped
forward deliberately: a ```lean fence is overwhelmingly Lean 4 in
practice.
highlight.rs -- `warning` moves from bold red to bold BRIGHT red. Writing
the test found the collision: `number` is plain `fg(1)`, so `sorry` and
the literal `42` beside it were the same colour, differing only in the
bold flag. `sorry` means "admitted, not proved" and is the one token in a
proof file a reader must never skim past, so it now gets the loudest
entry in the table and the test asserts the full style rather than the
colour.
Twelve criteria, seventeen tests. Notes on the ones that could have been
vacuous:
* acc4 uses a `.txt` fixture, not `.lean` -- on a `.lean` path the
extension alone yields `lean4` and the assertion would pass with the
alias table empty. acc4b removes the alias and pins that the raw name
survives, so acc4 cannot silently stop testing anything.
* acc11 goes through the real `_parse_now` injection path and asserts a
`lean4` CHILD LAYER appears. `pmacs.parse.injection_aliases` is a
documented write-only proxy, so an alias-table read would have proven
nothing about the parser; acc11b pins that a misspelled fence still
resolves to nothing.
* acc12 asserts through the process supervisor and the server list that
opening a Lean buffer spawns nothing. This is not decorative: the
machine this arc was scouted on has elan installed with no default
toolchain, where `lake --version` itself fails, and Stage 1 must be
unaffected by that.
Gates: fmt and strict workspace clippy clean; 1,826 default + 2,003 CRDT
library tests; lean4 Stage 1 9, comment toggle 14, auto-pair 45,
injection 4; M4 121; required GPU 152; isolated-config workspace sweep
3,150 across 90 suites; `git diff --check` clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`constructor`, `character`, `keyword.conditional`, and `warning` are the
captures the Lean query uses that the global theme table lacked. Three of
them are not Lean-only, so this is a deliberate retro-paint of already
shipped languages -- the #146 lesson applied on purpose rather than
discovered afterwards.
The blast radius, measured rather than assumed:
* `constructor` reaches SEVEN language entries, not four. The emitting
crates are rust, lua, python and javascript, but
`tree_sitter_javascript::HIGHLIGHT_QUERY` is concatenated base-first
into javascriptreact, typescript and typescriptreact as well.
* Its shape is not "constructors". rust/python/javascript tag every
capitalized identifier (`#match? "^[A-Z]"`); lua tags every
table-constructor brace. So this recolors `None`, every class-cased
name, and every Lua `{}` -- all of which rendered as unstyled default
text before.
* `character` reaches zig only; `keyword.conditional` reaches cmake and
zig, which previously flattened it to `keyword`; `warning` reaches no
other grammar and exists for Lean's `sorry`.
The alternative was an in-repo overlay renaming the captures (the #144
LaTeX pattern), which forks a 213-line query we would then own and
hand-merge on every crate bump. There is no middle option: styling Lean's
constructors without touching the other seven entries requires renaming
the capture, which requires the overlay.
Pinned in both directions, per #146:
* the positive breadth pin asserts all seven entries emit
`@constructor` at the QUERY level -- chosen over per-fixture checks
because the base-query composition is the fragile part; if someone
stops concatenating the JS base into `typescript`, this fails while
any single-language fixture still passes;
* two grid pins prove the theme entry reaches painted cells, and a
third records that a variant in CALL position keeps `@function` --
the difference between "capitalized identifiers recolor" and "enum
variants recolor", only the first of which is true;
* the negative pin asserts ten languages (markdown, json, yaml, html,
css, c, cpp, go, toml, bash) emit none of the four names, with a
non-vacuity check that the same predicate finds each name where it
does occur.
Rev 1 of the framing named Lua and Python in that negative pin, which was
a self-contradiction -- both are retro-painted by `constructor`, so the
assertion would have been vacuous in the #155 R2 shape. Review round 1
caught it.
Full lib suite (1,824) and the required-GPU gate (152) pass unchanged, so
no existing assertion depended on these captures being unstyled.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds `arborium-lean` 2.18 and one `BUILTIN_LANGUAGES` entry, closing the
framing's open verification obligation on the crate choice.
Why this crate and not `tree-sitter-lean4` (Q#LN1): the latter depends on
`tree-sitter = "0.25"` directly rather than the shared
`tree-sitter-language` ABI crate, and `^0.25` excludes our 0.26, so it
would fork the graph exactly as the dead `tree-sitter-dockerfile` does.
It also exports only `pub fn language()` while its README advertises a
`LANGUAGE` const that does not exist, and its package `include` omits
`queries/` so it ships no highlights at all. `arborium-lean` uses
`tree-sitter-language 0.1` as its sole runtime dep, ships a pre-generated
ABI-15 parser plus scanner, and exports real query constants.
`cargo tree -d` reports no duplicate `tree-sitter`.
The entry is named `lean4`, not `lean` (Q#LN2): `ensure_server` passes
`LanguageEntry.name` through as the `didOpen` language_id, and the Lean
ecosystem's id is `lean4` -- `lean` is Lean 3, which is end-of-life. It
claims `.lean` only; `.olean` is a compiled binary and `.ilean` is JSON
metadata (Q#LN3).
Four tests. The load-bearing one is `lean4_grammar_loads_and_parses`,
which discharges the half of Q#LN1 that could not be settled by reading:
`arborium-lean` exports `const fn language() -> LanguageFn` rather than
the `LANGUAGE` const every other entry uses, and its README demonstrates
usage against a patched tree-sitter core. Neither is supposed to matter,
but "supposed to" is not evidence. The fixture parses without error, and
-- the part that actually guards a misbuild -- its Unicode operators
produce structure rather than degrading silently: the grammar must see a
`(arrow)` for the arrow, a `(forall)` for the universal quantifier, and a
`(comparison)` for the inequality.
The error-free claim is deliberately scoped to the committed fixture.
Lean's syntax is user-extensible via macros, so a static grammar
mis-parses some legal input by construction; the framing scores that as
bet 3 rather than the doc overselling it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The approved framing for Arc 8, revision 4, after three review rounds.
Seven stages: grammar/mode, multi-root LSP affinity, the Lean language
server, the Unicode input method, the goal view, the #eval output
channel, and module hierarchy. 19 decisions, 64 acceptance criteria.
Committed as this branch first commit per the house workflow; the
implementation of Stage 1 follows.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`dominant_line_shape` averages only the lines in a bucket that have
content, then guarded the result with `bool::then_some`. `then_some`
takes its argument by value, so the `MinimapLineShape` literal --- and
with it `indent_sum / count` --- is evaluated before the `count > 0`
guard is ever consulted. When a bucket holds no contentful lines the
division panics and takes the GPU frontend down.
This is reachable in ordinary use, not at an edge: the bucketing branch
runs whenever a file has more lines than the minimap has pixel rows, and
it is exactly then that a run of blank lines can fill a whole downsampled
row. A whitespace-only line counts as blank too --- `minimap_line_shape`
subtracts the indent from the total, so `content_cols` is zero.
Switch to `bool::then`, which defers the body into a closure so the zero
case short-circuits to `None`. The call site already treats `None` as
"draw no stroke for this row", so no other change is needed.
Three tests, two of which fail against the previous line:
* a 10,000-line all-blank file driven through `minimap_rects`, which
reproduces the original panic through the real downsampling path;
* `dominant_line_shape` on an empty bucket;
* a mixed bucket, asserting the average still ignores blank lines ---
a companion guard so the fix cannot regress into counting the whole
slice.
A comment records why this must not be "simplified" back: clippy's
`unnecessary_lazy_evaluations` pushes in precisely the wrong direction
here, and does not fire on a body that can panic.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
PR #155 review round 2, self-review of the round-2 commit.
The round-2 change labelled "minor" — resolving both arms of
pmacs.window.buffer() through the acting frontend for uniformity — made
the NO-ARGUMENT arm fallible. `acting_frontend` follows the interactive
origin, which can name a frontend that has no registered view: a bare
`dispatch_key` from an unattached peer does exactly that. `selected_window`
then raises "acting frontend has no layout" instead of answering.
Nothing surfaced that error, because the runtime callers do not pcall it.
killring, syntax, autosave, pair, indent and comment all read
pmacs.window.buffer() on ordinary edits, so the raise silently dropped
the operation: kill_ring_acceptance went 30/30 to 25/5, with
frontend_detached_drops_per_frontend_state reporting only "B has kill
state". main is 30/30, and reverting this one file restored it.
The no-arg arm is back on ambient active_buffer_id() and now documents
why that is deliberate rather than an oversight: dispatch sets
active_frontend to the acting frontend before running a command, so the
two agree on every real path, while only the ambient resolver has the
fallback that makes it total. The explicit-window arm keeps its Q#BP11
layout validation, which is what the arc actually needed.
acc19c pins it through the real path — a buffer.after-edit subscriber
reading pmacs.window.buffer() during a viewless peer's dispatch_key —
rather than by calling the binding directly. Bite-verified:
scripts/bite bbe4152 src/lua_bindings/mod.rs --test
bottom_panel_stage1_acceptance -- acc19c goes red with the exact
"acting frontend has no layout" traceback.
The ledger also records two gating facts found on the way: the workspace
sweep must run with an isolated XDG_CONFIG_HOME, because the real user
init.lua installs a local package and the losing race leaks a status
message into painted-frame comparisons; and a latent pre-existing main
bug in the buffer CRDT undo path, which is not this branch's and whose
proptest seed is deliberately not committed here.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012j4omtTMn9v1UfmHQb9ap6
The round-2 peer press landed in the peer's CONTENT area, so it never
reached `arm_window_drag` — the exact path Finding 5 names — and the
case bit nothing. It now presses the peer's own mode-line row, where a
single global drag slot is overwritten (and, since that lone window owns
no boundary, cleared outright).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
PR #155 review round 2.
Finding 1 (must fix): Q#BP7 item 1 — "growth reaching the live tail
re-arms follow (top -> None), only when no selection is active" — was
never implemented. `at_bottom` is the instantaneous geometric readout
`scroll_offset == 0`, which a still-anchored view satisfies whenever it
happens to be tall enough to reach the tail, so the round-1 assertion
could not see the gap: the next rows the child printed pushed the
anchored view back into history.
`rearm_follow_on_growth` now clears `top` when a viewport-size
declaration makes the view cover the tail and no selection is frozen,
and every size-declaring path (`snapshot_for_view`, `record_view_size`,
`view_status_for_size`) routes through one `declare_view_size` helper so
grid and semantic declarations cannot disagree. `scroll_view` and
`begin_selection` deliberately stay out: they write `top` themselves,
and `scroll_view` already owns the scroll-driven arm.
New acc32b is the pin the review asked for: scroll into history, grow
past the tail, then release a SECOND burst of child output through a
filesystem gate and assert the view moved with it.
Finding 2: the PTY fixtures emitted LF-only output, which staircases
rightward until every row clips to blanks past the viewport width — so
the round-1 anchor assertions compared "" with "" and could not fail.
Both fixtures now emit CRLF, and each anchor comparison is guarded by
`assert!(!top_before.is_empty())`.
Finding 3: acc33's contrast case asserted nothing, and the behavior it
claimed was false as coded. With the re-arm in place it is true and now
asserted: clearing the selection at the same geometry re-arms follow and
leaves the frozen anchor.
Finding 4: `start_run` gated the panel branch on `display == "panel" or
already_in_panel(..)`, so an explicit `display = "current"` lost to the
inference — and that value is the documented user-facing opt-out from
the Stage 3 default flip. Now gated on OMISSION. acc19b gains the
explicit-"current" case.
Finding 5: `window_drag` is a `HashMap<FrontendId, WindowDragState>`, so
a peer's mode-line press can no longer steal or clear another
frontend's in-flight gesture, and concurrent drags are legal. Cleared on
detach. acc30c gains the mode-line-press case.
Minor: `pmacs.window.buffer()` resolves both arms through the acting
frontend using the shared `lookup_window` / `selected_window` validators
rather than re-implementing them beside an ambient `active_buffer_id()`.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`TerminalViewStatus.scroll_offset` is the retained rows between the
VIEWPORT and the live tail, so it necessarily tracks viewport height: an
assertion that it survives a panel height change unchanged is either
vacuous or wrong, and it went red once under a loaded sweep for exactly
that reason. Q#BP7's invariant is that the ANCHOR is frozen, so acc32
and acc33 now compare the first visible row's text across the change,
and additionally pin the follow behavior that distinguishes them: a
shrink never re-arms follow, growth reaching the tail does, and growth
with a frozen selection does not.
Both also wait for the child's last line before sampling, so neither
races further output.
Also records the round in docs/active-work.md.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
PR #155 review round 1.
Finding 1 (must fix): `try_split_active` had no production caller —
`pmacs.window.split_horizontal` / `split_vertical`, and therefore
`C-x 2` / `C-x 3`, still went through plain `split_active`. Splitting a
focused panel made the root wrapper's final child a split rather than
`Leaf(side)`, which both `Layout::compute`'s fixed pass and
`document_subtree` key on: the panel band reverted to 1:1 weight
division and an ordinary window ended up living inside it. Both bindings
now route through the guard, and acc26 asserts through the real Lua
path — a direct core call passes with the guard unwired, which is how it
survived the first round.
Finding 2: the armed-drag early return now checks the arming frontend,
so one frontend's in-flight gesture cannot cancel or swallow another's
mouse events. New acc30c.
Finding 3: `paint_mode_line_graphemes`'s doc block was left heading
`paint_divider_segment`; moved back.
Finding 4: a recompile carries no `display`, so it took the raw switch
and duplicated a panel-placed `*compilation*` into the document window.
`start_run` now detects that the buffer already owns the panel slot.
`pmacs.window.buffer` gained an optional window argument so an adopter
can ask without selecting the panel first. New acc19b.
Stage-2 hazard pins the review asked for, both in `src/daemon.rs`:
a fresh attach while LOCAL is focused in a panel inherits LOCAL's
document buffer, and an initial-target bootstrap whose `after-load`
hook creates and selects a panel still reasserts into a document window.
Minor: dropped listview's dead `p.side`; documented `focus_window`'s
caller-validates contract; `jump_back` restores through `focus_window`
so the "every focus change" contract holds; `params` / `resize` default
to the acting frontend's selected window rather than the ambient one;
widened the flexible-division math to u64 intermediates.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review finding, verified: rev 2's "no new build cost" was true only under
an unstated condition. ttf-parser re-exports `math` behind
`#[cfg(feature = "opentype-layout")]`, and it is compiled today only
because fontdb requests that feature — with `default-features = false` and
a set that is NOT ttf-parser's own default (fontdb's adds no-std-float and
omits std). A plain `ttf-parser = "0.25"` therefore unions std in and
forces a one-time rebuild of ttf-parser, fontdb, cosmic-text and glyphon.
Record the zero-rebuild spelling, `default-features = false, features =
["opentype-layout"]`, in Tier 3 §A and in the component table, so the
Tier 3 implementer declares it deliberately rather than tripping over it.
The C1 row points at the detail rather than repeating it.
Also note in the header that every anchor was re-checked at f07b75b. The
scout pin stays at ddaa80d because that is when the scouting happened;
#153 landed between the two and is test-only, moving no anchor cited here.
Framing only; no implementation, no runtime code.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both earlier attempts in this branch were wrong, and the diagnostic they
added is what proved it: the macOS failure reported a stable
`rendered prefix: 6/15 ("VTERM_")` BEFORE and AFTER the CRLF change, with an
identical cursor, across two completely different child layouts. Identical
truncation under different layouts cannot be a layout problem.
The real mechanism is pinned by the repository's own unit test,
`cell::tests::diff_split_by_unchanged_cell_is_two_spans`: `cell::diff` splits
a run at any cell where `prev == next` and never transmits that cell. So when
a character of the marker already happens to sit at its destination, the host
receives the marker with that byte MISSING, not merely escaped around. The
constant 6 is the distance to the first such hole.
That makes escape-stripped matching unsound in kind rather than merely
insufficient: no matching strategy recovers a byte that was never sent. It is
removed, and `wait_for_output` is strict again. What remains asserted through
host bytes are protocol escapes pmacs writes directly — the OSC 52 clipboard
reply, the alternate-screen and bracketed-paste resets — which are not painted
cells and which the differ never touches.
Readiness now gates on a file the child publishes, the pattern the reliable
sibling test in this file already uses. That the child's output reaches the
SCREEN stays asserted in-process over `snapshot_text`, at the layer that can
actually see it; this test keeps what it uniquely owns, the host lifecycle.
`strip_ansi` and `longest_rendered_prefix` are kept as failure diagnostics
only, and now carry a case pinning the dropped-cell shape so the wrong remedy
is not reached for again. The new readiness wait reports startup breadcrumbs
on timeout; the plain helper reports only the missing path, which is the least
useful thing to know at exactly that moment.
Both the readiness gate and its timeout diagnostic were falsified by pointing
the child at a path the test does not watch.
Test-only; no runtime code.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds the lane to docs/active-work.md: branch, base, what Stage 1
implemented, the verification run, and the two known local-only test
caveats (the parallel-load GPU flake and compile_mode_acceptance's
single-thread requirement).
The durable handoff snapshot stays untouched until the PR merges, per
its own update protocol.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Revision 1 was written against protocol v18, before LaTeX Stage 1 (#144),
web grammars (#146), folding Stages 1-2 (#142/#149) and the GPU initial
target (#148) landed. Revision 2 changes no design decision; it corrects
the ground truth those merges invalidated and records the staging decision
the sibling substrate framing already took. A new section 0 lists every
correction so a reader who knows revision 1 can read it alone.
Two corrections change implementation choices rather than line numbers:
- The MATH-table dependency story was wrong in both directions. Revision 1
said a crate must be added and that "neither is in the tree today";
ttf-parser 0.25.1 already reaches pmacs-gpu non-optionally through
fontdb -> cosmic-text -> glyphon, the same fontdb the frontend already
calls. And the choice is not "one of ttf-parser or read-fonts": only
ttf-parser exposes the MATH table, supplying exactly the constants Tier
3 names. read-fonts 0.37.0 is present but has none, so selecting it
would be a dead end.
- Tier 2's staging was already decided elsewhere and this note did not say
so. The sibling framing's Q#LX5 puts the parser beside its Tier 3
consumer, never ahead of it, because MathNode's shape is only validated
by a layout consumer. That makes Tier 2 not independently shippable,
which is worth stating explicitly: it is pure and conflict-free, so
landing it alone while other lanes hold the render path is exactly the
tempting move Q#LX5 refused.
Tier 1 is materially de-risked: the LaTeX grammar already exposes
math_environment and math_delimiter, and the in-repo query overlay this
tier proposed already exists and captures both, so the mechanism is proven
rather than speculative. The guessed node name (math_expression) is
corrected to the grammar's own. Markdown still needs the overlay
treatment.
Tier 4 gains a contention note. Revision 1 described the GPU render path
as though math were its only claimant; folding Stage 3 and the
bottom-panel arc's Stage 2 now converge on it, so whichever lands second
re-scouts against the first.
Framing only; no implementation, no runtime code.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- `listview.open`, `compile.run`, and `pmacs.terminal.open` all take the
same strict `display = "current" | "panel"`, validated before any
buffer, session, process, or wrapper exists. Omission keeps today's
behavior; Stage 3 flips the default.
- `listview.quit` / `compile.quit` delegate to `window.quit` only when
the buffer really is in a side window, so the presentation is deleted
or restored instead of leaving a source buffer stranded in the slot.
- LSP `visit_location`, LSP go-to-definition, and compile `visit_error`
route through `display_file`, so a visit from a panel lands in the
document target and fires its hook with that window active.
- `window.quit`'s Delete arm focuses the revalidated remembered origin.
- Capability fallback discards an accompanying `height` rather than
rejecting the call.
- `window.min-height` clamps a below-floor value on read instead of
refusing the write.
- `tests/bottom_panel_stage1_acceptance.rs`: 42 tests over the framing's
Stage 1 criteria, including the two production `Layout::compute`
callers, the recursive minima, hide/reappear, the final-focus matrix,
quit chains at the depth cap, per-frontend jump origins, the divider,
and a real-PTY pin of Bet B1.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The diagnostic added in the previous commit answered the question on its
first macOS run:
rendered prefix: 6/15 bytes ("VTERM_") — child text rendered only partially
So the child wrote and the host received part of the marker, but stripping
escapes did not rejoin the rest: other repainted cells sit between the two
pieces, not just cursor moves.
Six characters is exactly what fits before the right margin of this
session's 40-column child. The probe writes bare `\n`, and the supervisor's
PTY trampoline runs `stty raw`, which clears OPOST — so a lone `\n` moves
down without returning to column 1 and every line staircases five columns
right. After twenty lines the marker starts in the right margin, wraps
mid-word, and reaches the host as two pieces that no contiguous match can
join.
That also explains the intermittency: the wrap column depends on whether
pmacs has already resized the PTY from the requested 40 columns to the
window width, which races the child's first writes.
Write explicit carriage returns so every line returns to column 1 and the
markers start there. The fixture was wrong about its own line discipline;
the emulator was behaving correctly throughout.
The escape-stripped matching and the rendered-prefix diagnostic from the
previous commit are kept: they are what produced this answer, and they keep
the next such failure legible.
Test-only; no runtime code.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Stage 1 substrate for the bottom-panel arc (docs/bottom-panel-framing.md).
- `WindowParams` (side / fixed_rows / dedicated + implementation-owned
quit action and remembered document origin), `Side`, `QuitAction` with
a bounded replacement history, and the `MIN_WINDOW_OUTER_ROWS` floor.
- `Layout::compute(area, fixed)` allocates fixed rows before dividing the
remainder by weight; both production callers feed the same shared map,
including the peer-presence overlay pass that derives its own rect.
- `subtree_min_rows` / `interactive_min_rows`: the recursive minima, and
`boundary_below` for the shared drag / keyboard resize boundary rule.
- `FrontendView` gains `panel_capable`, `frame_geometry`, and the derived
`panel_hidden`, each spelled explicitly at every construction site.
- `EditorCore`: `primary_document_window`, the non-side target rule,
`display_buffer` + placement policy, `quit_window`, side-window removal
on `kill_buffer`, per-frontend jump entries with origin windows, and the
shared resolve/load-without-switch seam the initial-target bootstrap now
uses too.
- `EditorState`: the panel reconciliation transaction, geometry
declaration, the side-window `dispatch_idle_for` gate, divider paint,
and divider drag.
- `pmacs.window.display / display_file / quit / panel / params /
set_params / resize / display_target`, plus `builtin/runtime/window.lua`
with `window.panel-height`, `window.min-height`, and the resize commands.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The macOS `VTERM_ALT_READY` flake finally reported itself through #151's
breadcrumbs, identically in two runs: pmacs still running, `init.lua`
reached, `terminal.open` ok, and a settled screen whose tail is nothing but
`\x1b[22;42H` repeated 133 times.
That cursor is the evidence. A blank terminal parks at 1;1. Column 42 is
where the cursor lands after writing a 15-byte marker that ends at column
41 — so the child DID write and the emulator DID receive it. What failed
was the assertion: `wait_for_output` required the needle to appear as
contiguous bytes, but the TUI differ paints only changed cells and skips
ones that already match, so a run held contiguously on one screen row can
still reach the host as `PREF<cursor-move>IX`.
Match over escape-stripped bytes when the needle is plain text. This cannot
mask the failure that matters: text the child never wrote is absent from
the stripped stream too, so a genuinely silent child still fails. Needles
carrying their own escape (the OSC 52 clipboard reply) keep the strict path,
since stripping would consume the bytes under test.
The failure arm now also reports how much of the needle rendered, so the
next occurrence distinguishes "nothing reached the host" — a PTY/spawn
fault — from a partial render, instead of leaving a tail of pure escapes
that cannot tell them apart.
Both helpers are pinned directly, including that stripping rejoins a split
run without inventing absent text. `strip_ansi`'s first draft mishandled
`ESC ( B`, whose intermediate byte makes it three bytes rather than two;
its test caught that.
Test-only; no runtime code.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds the approved framing for the bottom-panel arc: a buffer displayed
in a fixed-height window pinned to the bottom of the frame, targeted by
policy rather than by stealing the selected window, plus the missing
display-buffer/window-parameter concept underneath it.
Revision 4 follows three review rounds, an integration review, and a
landed-state audit against GPU initial target (#148, protocol v20) and
folding Stage 2 (#149).
Amended before branching by the pre-implementation dependency
verification recorded in section 0.6:
- the folding dependency is cleared and re-verified against canonical
main at ddaa80d (nothing in flight, folding Stage 2 acceptance 48/48
green, every borrowed anchor reproducing, and folding's only window.rs
edit confined to one 22-line hunk that leaves the layout functions
pre-folding code);
- R5-B1: Layout::compute has TWO production callers, not one. The
second, the peer-presence overlay pass in src/overlay_paint.rs, builds
its own text-area rect from active_layout() and never routes through
window_placements, so the planned compute(area, fixed) signature change
would otherwise leave every peer cursor painted at its no-panel row.
Corrected in section 1.1 and Q#BP2, pinned by acceptance 1, and the
fixed map is now specified to come from one shared helper rather than
being assembled per call site.
Stage 1 (window placement + TUI side windows) changes no wire shape.
Move the lane from active-work.md into its Closed section, retire the
protocol v20 / main-hash references to LANDED form in agent-handoff.md, and
record both review-round lessons (failure-socket containment, upgrade-gated
replica publication) in the ops-lessons ledger.
18 of the last 60 CI runs failed (30%), including repeatedly on `main`.
Every failure is macOS-only and hits BOTH Lua flavors, so it is the
runner, not LuaJIT. Sampling 7 showed only two tests.
**m8_9 outline budget** — `outline_5_level_100_entry_renders_within_100ms`
is a wall-clock budget observed at 147ms and 149ms against 100ms on
GitHub's shared macOS runners, while Linux lands comfortably under. The
measurement and its printout now always run; only the ASSERTION is gated
on `!cfg!(target_os = "macos")`, exactly as
`composition_overhead_under_ten_percent` already is in `src/editor.rs`
for the same reason. A real regression still surfaces on Linux, on the
perf gates, and in the number printed to the log.
**vterm PTY smoke** — deliberately NOT a timeout bump. Instrumenting
locally showed the failing wait completes in 40ms against a 10s budget
(250x headroom), while the genuinely tight wait in the same test (2.86s
against 5s) never fails. Four hypotheses were eliminated with evidence:
- python3 cold start: the sibling test at :494 spawns the same
/usr/bin/python3 with a TIGHTER 5s budget and passes in the very
runs where the smoke fails ("3 passed; 1 failed");
- config resolution: XDG_CONFIG_HOME is read straight from the env
(src/config.rs:49), no platform branch;
- a stalled idle loop: the run loop polls on a 60Hz frame timeout and
ticks the supervisor every frame (src/editor.rs:2522), so child
output drains without input;
- a too-small budget: see the 250x headroom above.
The real defect this commit fixes is that NONE of those could be
distinguished from the CI log, which carried only "host output never
contained VTERM_ALT_READY" plus a tail of escape bytes. init.lua now
writes breadcrumbs (reached / terminal.open ok-or-error) and the timeout
reports whether pmacs is still alive plus each breadcrumb, so the next
occurrence names its own cause:
startup: pmacs still running; init.lua reached="1"; terminal.open="ok"
Verified non-vacuous: forcing the needle to never appear produces the
line above, which also proves io.open works in pmacs's Lua — otherwise
the breadcrumbs would silently read MISSING and mislead.
Test-only; no runtime code.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q5BkezMppbpCgGAYk2ftxV
Integrate folding Stage 2 and its landed-state documentation with the
protocol-v20 GPU initial-target branch. Preserve per-session fold projection
selection in the target bootstrap transaction and retain v19 compatibility
coverage after the later protocol bump.
Shut down bootstrap sockets on every dispatcher-side failure and reject
frontend events whose session state was never installed. This prevents a
lingering failed client from reaching absent render/size state.
Track target-side CRDT upgrades independently from load/create status so a
deduplicated hidden buffer is published to every existing grid replica. Add
real-daemon regressions for both failure containment and replica publication.
Levi Neuwirth