docs: record dired Stage 1 review round 1
Framing rev 7 adds S1-10..S1-12 -- the three findings that changed behavior, each stated as the durable lesson rather than as a diff: painting takes a buffer and seating takes the world, so any post-await cursor operation needs an active-buffer guard; the rendered columns are a contract Stage 3 is planned against, so precision yields to width; and `open_directory`'s changed-nothing-on-failure invariant is itself a probe, which is why the symlink descent no longer lists the target twice. Plus the tolerant-channel note: cancellation was never a backstop for a dired listing, because nothing cancels one. The ledger records the round, the updated counts (dired 25 + 25 CRDT, sweep 3,189 across 92), and the process lesson that cost me the fixes once: a mutation-bite helper restores with `git checkout --`, so a fix must be committed before it is bitten.
This commit is contained in:
parent
531fdf404e
commit
14881b26c0
|
|
@ -206,10 +206,26 @@ If it does not, stop and repair the remote/fetch configuration.
|
|||
fail the test that names it. `dired.lua` is new, so `scripts/bite`'s
|
||||
file swap does not apply; every mutation was applied and reverted with
|
||||
`git checkout --`. One came back VACUOUS and is recorded above.
|
||||
- **Review round 1 addressed** (framing rev 7, S1-10…S1-12). Three
|
||||
behavioral fixes, each bite-verified: `dired.revert`'s re-seat is
|
||||
guarded on the active buffer (an ambient `move_to_line` after an await
|
||||
moved an unrelated buffer's cursor — the buffer-level instance of
|
||||
S1-9); `fmt_size` keeps the column width past ten digits, because
|
||||
`_layout` is a contract Stage 3 is planned against; and the symlink
|
||||
descent dropped its probe, since `open_directory`'s
|
||||
changed-nothing-on-failure invariant *is* the probe (it was listing the
|
||||
target directory twice). Plus a consecutive-`readdir`-error cap, because
|
||||
**nothing cancels a dired listing** — it carries no supersede key, so
|
||||
cancellation was never the backstop the tolerant loop implicitly relied
|
||||
on. Naming/comment findings taken as-is.
|
||||
- Durable process lesson, hit twice now: a mutation-bite helper restores
|
||||
with `git checkout --`, which reverts to **HEAD** — so a fix must be
|
||||
committed *before* it is bitten. Round 1's fixes were briefly wiped by
|
||||
exactly that.
|
||||
- Verification on this branch: `cargo fmt --check` clean; strict workspace
|
||||
Clippy clean; 1,829 default + 2,006 CRDT library tests; dired acceptance
|
||||
22 default + 22 CRDT; m8_1 10 / m8_2 15 / m8_3 32 unchanged; M4 121;
|
||||
required GPU 155; **isolated-`XDG_CONFIG_HOME` workspace sweep 3,186
|
||||
**25 default + 25 CRDT**; m8_1 10 / m8_2 15 / m8_3 32 unchanged; M4 121;
|
||||
required GPU 155; **isolated-`XDG_CONFIG_HOME` workspace sweep 3,189
|
||||
passed across 92 suites, zero failures**; `git diff --check` clean. The
|
||||
sweep needs the isolated config for the reason recorded in the
|
||||
bottom-panel lane below.
|
||||
|
|
|
|||
|
|
@ -1,13 +1,14 @@
|
|||
# Dired — framing
|
||||
|
||||
**Revision 6 — 2026-07-25. Status: APPROVED; Stage 0 MERGED as #162;
|
||||
Stage 1 IN REVIEW as PR #165.**
|
||||
**Revision 7 — 2026-07-25. Status: APPROVED; Stage 0 MERGED as #162;
|
||||
Stage 1 IN REVIEW as PR #165, review round 1 addressed.**
|
||||
Rev 1 passed a ground-truth review; rev 2 fixed round 1's seven findings;
|
||||
rev 3 fixed round 2's six and was approved; rev 4 recorded what Stage 0's
|
||||
implementation falsified in the approved text (§0); rev 5 adds the
|
||||
**coherence impact** statement now required of every framing
|
||||
(`CLAUDE.md`, `COHERENCE.md` §20) — see §0.5; rev 6 records what Stage
|
||||
1's implementation falsified (§0, S1-1…S1-9). Deliberately
|
||||
1's implementation falsified (§0, S1-1…S1-9); rev 7 adds what its first
|
||||
review round found (§0, S1-10…S1-12). Deliberately
|
||||
unnumbered: the roadmap's Arc 8 is GPU
|
||||
structural parity but `docs/lean4-mode-framing.md` also claims Arc 8, so
|
||||
the arc space is already forked in uncommitted work. (Rev 2 also cited
|
||||
|
|
@ -293,6 +294,47 @@ in the code, per the rev-4 precedent.
|
|||
surface takes no frontend argument) and named here rather than
|
||||
discovered later.
|
||||
|
||||
### Stage 1 review round 1 (rev 6 → rev 7)
|
||||
|
||||
Three findings changed behavior; the rest were naming and comments. Each
|
||||
fix is bite-verified against the test that names it.
|
||||
|
||||
- **S1-10. An ambient re-seat is not safe after an await.** `dired.revert`
|
||||
painted its own buffer by name (safe) and then re-seated through
|
||||
`pmacs.editor.move_to_line`, which moves whatever window is
|
||||
**active** — so a user who switched buffers while the re-read was in
|
||||
flight had an unrelated buffer's cursor moved to a line index
|
||||
meaningful only in the dired listing. This is the buffer-level instance
|
||||
of the hazard S1-9 named at the frontend level, and it generalizes: in
|
||||
this codebase, *painting takes a buffer and seating takes the world*.
|
||||
Any post-await cursor operation needs an active-buffer guard;
|
||||
`open_directory` is exempt only because it displays the buffer first.
|
||||
- **S1-11. The rendered columns are a contract, so precision yields to
|
||||
width.** `%10d` overflowed at 10 GB (VM images, core dumps), widening
|
||||
the size field and shifting mtime and name right on that line alone.
|
||||
Cosmetically harmless today, but `_layout` is exported and Stage 3's
|
||||
column-classifying intercept is planned against it, so a
|
||||
contract-violating line now is a Stage 3 trap. `fmt_size` took
|
||||
`fmt_mtime`'s shape: exact bytes while they fit, else a fixed-width
|
||||
magnitude. Not the deferred human-readable column (§13) — the exact
|
||||
count still renders right up to the point where it cannot.
|
||||
- **S1-12. `open_directory`'s "changed nothing on failure" invariant is
|
||||
reusable as a PROBE.** S1-8's symlink descent originally listed the
|
||||
target to learn its kind and then opened it — two full listings of the
|
||||
same directory. Because a failed open touches no editor state
|
||||
(acceptance 15), the open itself is the probe: try the descent, fall
|
||||
back to `display_file`. One read. The comment that claimed "one
|
||||
syscall" for a full `read_dir` is corrected rather than left as a
|
||||
cost claim nobody would re-check.
|
||||
|
||||
Also, on the tolerant channel (Q#DR6): a `readdir` iterator may keep
|
||||
yielding errors without terminating, and **cancellation is not a backstop
|
||||
for a dired listing** — it carries no supersede key, so nothing cancels
|
||||
it. A consecutive-error cap now fails the listing the way an unopenable
|
||||
directory fails, rather than accumulating error rows on a worker thread.
|
||||
It is deliberately untested: faking a failing iterator would need the
|
||||
walk generic over it, a refactor with no other consumer.
|
||||
|
||||
## 0.5. Coherence impact (`COHERENCE.md` §20)
|
||||
|
||||
Required of every framing since #163. This arc was scouted and approved
|
||||
|
|
|
|||
Loading…
Reference in New Issue