d9b10905d0
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
226 lines
13 KiB
Markdown
226 lines
13 KiB
Markdown
# The headless CLI
|
|
|
|
`improvise cmd` and `improvise script` are one function — `run_headless_commands`
|
|
in `src/main.rs`; `run_headless_script` reads the file and delegates. Every claim
|
|
below therefore holds for both, and a fix to one is a fix to both by
|
|
construction rather than by remembering.
|
|
|
|
## 1 — the status line is the only channel, so it must be printed
|
|
|
|
`App` has no return channel for "what happened". A command's outcome — a
|
|
refusal, a warning, a `fit-measure`'s reproducing seed — is written to
|
|
`ViewState::status_msg` and rendered by the TUI's bottom bar. Headless, nothing
|
|
read it, so **success and refusal were byte-identical: silence and exit 0**
|
|
(improvise-q914, improvise-j6x8). Two things followed that are worth keeping
|
|
in mind, because both were mistaken for correct behaviour by agents working
|
|
above this layer:
|
|
|
|
- A P1's carefully-worded refusal can be written, tested at the effect, and
|
|
still reach nobody, because the test asserts the *effect's* status and the
|
|
surface that would show it drops it. That is this repo's characteristic
|
|
defect one layer out, and the only cure is a test at the process boundary —
|
|
`tests/headless_cli.rs`, which spawns the real binary.
|
|
- `improvise cmd 'fit-measure …'` looked like it did nothing whether it worked
|
|
or not, which made the whole GP feature read as broken from a script.
|
|
|
|
The loop takes the message with `std::mem::take` and clears it per command, so
|
|
one command's status cannot be reported twice or attributed to its successor.
|
|
|
|
## 2 — stderr, all of it, and why no split was possible
|
|
|
|
The tempting design is stdout for a report and stderr for a refusal. **It
|
|
cannot be built honestly**, because `status_msg` is a bare `String` with no
|
|
severity, and both candidate classifiers are disproved by code in the tree:
|
|
|
|
- *"A refusal is a batch that aborted."* Only three effects set
|
|
`abort_effects` (gotcha 21). `AddItem` writes `workbook.add_item`'s `Err`
|
|
straight to the status and does not abort — and that is q914's own worked
|
|
example of a refusal. So this classifier puts a refusal on stdout labelled a
|
|
success.
|
|
- *"A refusal is anything that spoke."* `cmd/fit.rs` sets a status on
|
|
**success**, carrying the seed that reproduces the fit. So this one labels a
|
|
deliverable a failure.
|
|
|
|
Guessing between them would be precisely the adjacent-check defect
|
|
`context/design-principles.md` §7 names. So everything goes to stderr: it is
|
|
the diagnostics stream, it leaves stdout free for a future data output, and it
|
|
matches every other diagnostic `main.rs` already emits (`Saved to …`,
|
|
`warning: …`, `Parse error: …`).
|
|
|
|
**No `--quiet` flag.** Silence is never the default and `2>/dev/null` is the
|
|
standard suppression; a flag would be a second knob with nothing asking for it.
|
|
|
|
## 3 — the exit code, and the hole left open deliberately
|
|
|
|
Exit 1 on a parse error (pre-existing) or on `App::last_batch_aborted()`. Both
|
|
are real flags read off state the code already keeps, so neither is a guess.
|
|
|
|
What this does **not** cover, stated rather than papered over: a refusal that
|
|
only writes a status still exits 0. Closing it needs a severity beside
|
|
`status_msg`, which lives in `ViewState` and is therefore a wire change
|
|
(gotcha 25) reaching the browser client — out of proportion to the CLI, and
|
|
filed as improvise-udt2, which names both rivals so the next attempt cannot
|
|
rediscover them. Until then the README says plainly that exit 0 means "nothing
|
|
aborted", not "everything worked", because a contract that over-promises is
|
|
worse than a narrow one.
|
|
|
|
## 4 — a run that changed nothing does not write
|
|
|
|
`persistence::save` used to run unconditionally, so `improvise cmd` normalised
|
|
the document on every invocation — `1e20` expanded to twenty digits. The cost
|
|
was not cosmetic: an agent editing a fixture by string-replacing a value after
|
|
one run got a silent no-op, and two misleading test results out of it.
|
|
|
|
The gate is `changed`, accumulated from `model_state.dirty` **after each
|
|
command**, not read once at the end. Reading it at the end is wrong because
|
|
`effect::Save` clears the flag, so `[add-item …, w other.improv]` would look
|
|
unchanged and drop the edit from the `-f` document. `!path.exists()` is or-ed
|
|
in so that naming a new file still creates it.
|
|
|
|
Over-approximation is the safe direction here, the same one `Dirty::of` takes
|
|
(gotcha 35): a `Model(_)` effect that happens to write the value already there
|
|
still costs one redundant save.
|
|
|
|
## 5 — the refusal names the word the user typed
|
|
|
|
`CmdRegistry::resolve` maps `formula` → `add-formula` *before* the arity check
|
|
runs, so every validator that spelled its own name reported a command nobody
|
|
typed (improvise-z05h). The fix moves the name to the one place that knows
|
|
which of the two the user wrote: `require_args` no longer takes a `word`, and
|
|
`CmdRegistry::parse` prefixes `"{typed}: "` onto any `Err`.
|
|
|
|
That removed 41 name literals from `cmd/`, which is the same argument gotcha 15
|
|
makes about `registry.rs` having no name literals — a name spelled twice can be
|
|
spelled wrong once. `interactive` deliberately does **not** prefix: `dispatch`
|
|
in `command/keymap.rs` already formats `"{name}: {e}"` with the *bound* name
|
|
(improvise-55l5, PR #126), and prefixing in both would double it.
|
|
|
|
The message states the **shape** rather than the arity, because
|
|
"requires 1-2 argument(s), got 6" does not tell anyone to quote anything.
|
|
|
|
## 6 — the help's ex-commands were the unguarded surface
|
|
|
|
`help.rs` derives `HelpLine::Keys` rows from the keymap and drops any row whose
|
|
command does not resolve, so a key row cannot go stale. `HelpLine::Literal` and
|
|
`HelpLine::Accent` rows are hand-written prose and had nothing checking them —
|
|
which is how all three documented spellings of `:formula` came to be ones the
|
|
parser refuses.
|
|
|
|
`every_ex_command_the_help_pages_show_is_one_the_parser_accepts` closes that
|
|
surface rather than the one instance: every help line whose trimmed text opens
|
|
with `:` is split on its colons and each piece handed to `parse_line`. The
|
|
split exists because one row shows two examples side by side.
|
|
|
|
What it still cannot see, and should be closed if the welcome text moves again:
|
|
`draw.rs::draw_welcome` builds its lines as a local array inside a ratatui
|
|
render function, so nothing can walk them. Its `:formula` line was corrected by
|
|
hand.
|
|
|
|
## 7 — a script's command is built against the cursor it will run against
|
|
|
|
`improvise cmd` and `improvise script` used to resolve every command through
|
|
`CmdRegistry::parse` — the **pure** arm, which builds `CursorState::default()`:
|
|
row 0, col 0, and a grid of zero rows and zero columns. The interactive arm,
|
|
which builds `CursorState::from_ctx(ctx)`, was never reached from a script.
|
|
|
|
So every `Move` (`move-selection`, `scroll-rows`, `page-scroll`, the four
|
|
`jump-*`) and `enter-advance` snapshotted a zeroed cursor at **parse** time,
|
|
clamped to `(0, 0)`, and emitted `SetSelected(0, 0)`. That is worse than a
|
|
no-op: it *resets* the cursor. Nothing could position it, so every
|
|
cursor-reading command — `clear-cell`, `yank`, `paste`, `cycle-axis-at-cursor`,
|
|
`hide-selected-row-item`, `edit-or-drill`, `drill-into-cell`,
|
|
`toggle-group-under-cursor`, the three `delete-*-at-cursor`,
|
|
`commit-and-advance` — was pinned to the origin (improvise-6uva). It was
|
|
introduced with `CursorState` and the two-arm registry in April and predates the
|
|
browser work by five months.
|
|
|
|
**A registry with two construction arms silently demotes every caller of the
|
|
weaker arm, and nothing type-checks which arm a caller deserves.** That is the
|
|
shape to remember, and it has a second instance in the same fix: the
|
|
`register_pure` entries had an interactive arm that refused **empty** args while
|
|
their pure arm accepted them, so `:w` (save to the current path) parsed one way
|
|
and refused the other. Both arms are now `parse`, which is what "needs no
|
|
interactive context" was supposed to mean;
|
|
`a_pure_commands_two_arms_agree_on_no_arguments` is the guard.
|
|
|
|
The repair is `parse::segments` plus `parse::parse_segment_interactive`, and the
|
|
loop re-parses **per segment**, because command *n* must see command *n-1*'s
|
|
cursor — which means construction cannot be hoisted out of the loop. Line
|
|
atomicity is preserved by keeping the existing whole-line `parse_line` as a
|
|
**syntax gate** before anything in the line runs: a line that parsed before
|
|
still parses, and a line that did not still does not, so the only behavioural
|
|
change is which cursor the commands see.
|
|
|
|
**That atomicity claim was false as written** (improvise-2bac), and §8 is what
|
|
makes it true. The gate runs the pure arm; the loop runs the interactive arm. A
|
|
gate is only a gate if *every argument list the pure arm accepts, the
|
|
interactive arm accepts too* — and `commit-cell-edit` broke it:
|
|
`add-category Foo . commit-cell-edit editing 5 Foo/x` on an empty document
|
|
passed the gate, applied the first segment, and then refused the second with
|
|
`no cell at cursor`. Half the line landed.
|
|
|
|
The TUI's `:` line had the same defect and is fixed the same way, with one
|
|
limitation that is not new: `ExecuteCommand` returns one effect batch, so the
|
|
segments of `:a . b` all see the pre-batch cursor, exactly as a
|
|
`Binding::Sequence` does (gotchas 21 and 23).
|
|
|
|
## 8 — the interactive arm is `parse`, then context; it is never a second parser
|
|
|
|
#151 fixed three instances of *a registry with two construction arms demotes
|
|
whichever caller reaches the weaker one*, one patch each. A post-merge review
|
|
found the fourth (improvise-2bac): `commit-cell-edit <mode> <value> <coords>`,
|
|
whose interactive arm read only `<mode>`, took the value from the `edit` buffer
|
|
and the key from the cursor. So a script that wrote 5 to `Region/East`
|
|
**deleted the cell under the cursor** — the empty buffer, committed — with exit
|
|
0 and nothing on stderr. The fifth was already in the tree:
|
|
`toggle-panel-and-focus <panel> false false` ignored both flags and toggled.
|
|
|
|
The shape rather than the instances is what changed. `CmdRegistry::register`
|
|
used to take two free functions, `parse(args)` and `interactive(args, ctx)`,
|
|
and nothing related them; each command re-derived its arguments twice and every
|
|
disagreement was silent. It now takes three pieces, and the interactive arm is
|
|
**built from `parse`**, not beside it:
|
|
|
|
- `parse(args) -> C` — the only function that reads arguments a caller supplied.
|
|
- `contextualize(C, &CmdContext) -> C` — fills the fields that are context and
|
|
never argument (a `Move`'s origin cursor, a `MovePanelCursor`'s bound). It is
|
|
handed the built command and **not the args**, so it cannot discard them.
|
|
- `fallback: Option<(args, ctx) -> C>` — reached **only** when `parse` refused.
|
|
This is where "no coordinates means the cursor" lives: `clear-cell` bare, and
|
|
`commit-cell-edit <mode>` bare, which is what the keymap binds.
|
|
|
|
Two consequences hold by construction rather than by test. **Whatever `parse`
|
|
accepts, the interactive arm accepts** — `contextualize` is infallible — so the
|
|
§7 gate cannot pass a segment the loop then refuses. And **an accepted argument
|
|
list cannot be overridden by the cursor**, because the only function that sees
|
|
both args and context never runs on an accepted list. `register_pure` and
|
|
`register_nullary` already had one arm; they are the `contextualize = identity,
|
|
fallback = None` case.
|
|
|
|
What is still representable, stated rather than hidden:
|
|
|
|
- `contextualize` can overwrite a field `parse` set. It would have to name the
|
|
field, which is a visible line in review, where the old `|_args, ctx|` was an
|
|
invisible omission. `commit_cell_edit_honours_an_explicit_value_and_cell_over_the_cursor`
|
|
fails if `commit-cell-edit`'s does.
|
|
- A `fallback` sees the args `parse` refused and could ignore some of them.
|
|
`commit-cell-edit`'s refuses anything but a lone mode, so the malformed
|
|
`commit-cell-edit editing 5` is an error on the `:` line — which has no pure
|
|
gate — rather than a commit of the edit buffer to the cursor cell.
|
|
- The gate is **stricter** than the loop wherever a fallback exists: a bare
|
|
`clear-cell` is refused in a script. That is the safe direction — the whole
|
|
line is refused — and it is unchanged from before #151.
|
|
|
|
`toggle-panel-and-focus` did not fit `contextualize`, because its pure arm
|
|
defaulted the omitted flags to `true` and a contextualizer cannot tell an
|
|
omitted flag from an explicit one. So the omission is now in the type —
|
|
`target: Option<(bool, bool)>`, `None` meaning *toggle* — and the toggle is
|
|
resolved in `execute` against the same context. That is the honest home: the
|
|
decision depends on context, and `execute` is where context is.
|
|
|
|
Guards: `every_argument_list_the_gate_accepts_the_run_accepts_too` enumerates
|
|
**every registration** against every candidate argument list, in a context whose
|
|
cursor sits on no cell — the context in which the old `commit-cell-edit` arm
|
|
refused. It lists no commands, so a new registration is covered the day it
|
|
lands. It is backed by the process-level tests in `tests/headless_cli.rs`.
|