Skip to content

fix(omp): read the ghost by colour CHANGE, not by an unstyled draft - #142

Merged
AltanS merged 2 commits into
AltanS:mainfrom
enieuwy:fix/omp-ghost-relative-colour
Aug 30, 2026
Merged

fix(omp): read the ghost by colour CHANGE, not by an unstyled draft#142
AltanS merged 2 commits into
AltanS:mainfrom
enieuwy:fix/omp-ghost-relative-colour

Conversation

@enieuwy

@enieuwy enieuwy commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

The stall is back on omp 18

#128 taught composerGhost that omp paints an inline completion suggestion after the operator's text, and that the suggestion is not in the input buffer. The rule it shipped was "the trailing run of COLOURED segments that follows UNSTYLED text", because on omp 17 the draft carried no foreground of its own.

omp 18.0.11 paints the draft while the agent is working. Live capture of the bottom border, after pane.send_text of list the files in this repo into a busy pane (omp 18.0.11, 2026-08-30):

ESC[38;2;250;81;53m╰─ ESC[38;2;242;244;248mlist the files in this repoESC[38;2;111;115;119mrtESC[0m … ─╯

The anchor is gone, so the rule refuses (fail-closed), the ghost stays in the draft, and extractInputDraft reads list the files in this report. draftCarriesSend requires containment, report is not contained, the submit key is withheld — and every reply to a busy omp pane stalls with "Message didn't reach the input box — a dialog may be waiting", exactly as before #128. An idle omp 18 pane still paints the draft unstyled, which is why this only bites the panes you most want to answer from your phone.

The fix

Make the rule relative on both sides: a ghost is the trailing run of segments sharing one foreground that differs from the text before it. The draft's own colour is then whatever precedes the run — present or absent — and both shapes read the same.

The three refusals stay, all fail-closed, because a wrongly-claimed ghost shortens the draft the reply guard verifies:

  • a row painted in one foreground end to end claims nothing;
  • a trailing run with no foreground claims nothing (omp's suggestion always carries one, and an unstyled tail after a coloured head is far more likely to be the operator's own text);
  • a run with nothing but blanks before it claims nothing.

Evidence

Measured with the production functions against the live captures above:

capture before after
busy pane + ghost draft list the files in this report, draftCarriesSend false draft list the files in this repo, true
idle pane + ghost draft list the files in this repo, true unchanged
355-char wrapped draft correct unchanged

New fixture omp--draft-ghost-suggestion-busy.txt, derived from omp--draft-ghost-suggestion.txt by splicing in the one SGR run omp 18 adds (no cell added or removed), filed into all three corpus lists. bun run test in web/: the 22 harness suites pass (2500 tests). The 8 failing files on this machine are the pre-existing localStorage-in-runner ones and reproduce on a clean tree.

Per the fork rule: no version bump, no CHANGELOG entry. Suggested line if you want one: omp 18 paints the composer draft while working, so the inline-suggestion guard now reads the ghost by colour change rather than by an unstyled draft.

omp 18 paints the composer draft in an explicit theme foreground while the
agent is working, so composerGhost — which claimed "the trailing coloured run
that follows UNSTYLED text" — found no anchor, left the inline suggestion in
the draft, and every reply to a busy pane stalled again with "Message did not
reach the input box".

The rule is now relative on both sides: the ghost is the trailing run of
segments sharing one foreground that DIFFERS from the text before it. Same
refusals, all fail-closed — a row painted in one foreground end to end, a
trailing run with no foreground, and a run with nothing but blanks before it
all claim nothing.
Review raised the trailing-colour heuristic eating text the operator typed.
Measured: omp paints its magic keywords as a per-character gradient, so a draft
ending in one loses its LAST character — and the previous rule was worse here,
taking all of `ultrathink` where this one takes `k`.

It cannot move a send verdict: draftCarriesSend accepts any contiguous run of
the draft's visible characters inside what was typed, so a contained draft stays
contained when a character comes off the end. What it costs is the stranded-draft
preview, one character. Tightening the other way — refusing a tail that changes
colour more than once — puts every @mention- and placeholder-ending draft back
into the permanent stall, so it is rejected on purpose and the trade is now a
pinned test rather than an accident.

Also: retarget the new fixture's welcome banner to the version it represents
(same length, no cell moved) and correct the all-LF list in the fixture README.
@enieuwy

enieuwy commented Aug 30, 2026

Copy link
Copy Markdown
Contributor Author

Review response: the over-claim is real, its harm chain is not, and the shipped rule was worse

A reviewer pass raised one P1 — "a partial pane read ending at a coloured typed token produces a shortened draft; draftCarriesSend accepts the remaining prefix and Enter fires early." I measured all three legs of that. The decoration is real; the harm chain does not hold; and the direction is backwards.

1. The decoration is real. omp 18 paints its magic keywords (ultrathink, workflowz) as a per-character colour gradient, and [Image #1] / [Paste #1] placeholders in the accent colour. A draft ending in one of those does lose its last colour run to this rule.

2. It cannot move a send verdict. draftCarriesSend accepts any contiguous run of the draft's visible characters inside what was typed (MIN_MATCH_CHARS = 8 floor aside) — deliberately, because the composer windows a long draft. So a partial arrival already passes before any ghost is subtracted:

draftCarriesSend(sent, "please walk the export clone end to")  → true   // untouched partial read
draftCarriesSend(sent, "please walk the export clone end t")   → true   // one character stripped

Over-claiming shortens a string that was already contained. It cannot turn a refusal into an acceptance for a partial read, because there was no refusal to turn. The only way stripping flips false → true is when the removed text was not typed — which is the ghost, i.e. the function working.

3. The shipped rule is worse in exactly this case. Both rules, same row, an unstyled draft ending in a gradient-painted keyword:

rule claims
main today (coloured run after unstyled text) ultrathink — the whole keyword
this PR (trailing run whose colour differs) k — one character

So merging reduces this exposure from ten characters to one; declining keeps the larger one and keeps every busy pane stalled.

4. What it costs, and why I did not tighten further. The residual cost is the stranded-draft preview: "Take over" can hand back a draft one character short. The obvious tightening — refuse a tail that changes colour more than once — kills the gradient case, but it also refuses every @mention- and placeholder-ending draft that has a real ghost after it, putting those back into the permanent stall. That is the failure the operator actually feels, so the trade goes the other way on purpose.

Pushed 0adbd3e on top:

  • the header note now states the over-claim, its bound, why it cannot reach a send verdict, and why the tightening was rejected;
  • a test pins it as a decision rather than an accident (composerGhost"k", and both drafts still satisfy draftCarriesSend);
  • the new fixture's welcome banner is retargeted to the version it represents (same length, no cell moved), and the fixture README's all-LF list is corrected to ten.

cd web && bun run test -- src/lib/harness: 22 files, 2501 tests green.

Two smaller review notes were correct and are fixed in the same commit; the remaining ones (ANSI var(--ansi-N) vs rgb(...) for the same visual colour under a custom theme, a ghost split across several foregrounds, a background-only ghost) are all in the refuse direction — they cost a stall, never a keystroke — and none is reachable from omp 18's hintStyle, which emits one foreground.

@AltanS

AltanS commented Aug 30, 2026

Copy link
Copy Markdown
Owner

Thank you — the live capture of the omp 18.0.11 bottom border is exactly the evidence this needed, and reading the ghost by colour change rather than by an unstyled anchor is the more durable rule. Appreciated you also pinning the bounded over-claim and saying what it costs. Verified against main with #140, #141 and #121: no conflicts, full suite green.

@AltanS
AltanS merged commit 4c1dde4 into AltanS:main Aug 30, 2026
1 check passed
AltanS added a commit that referenced this pull request Aug 30, 2026
Brings #140, #141, #142 and #121 across. Two of them needed a hand on the way
in, because git merged the text and not the meaning:

  · codex/markers.ts — #141 was written against main's `CONTROL_CHARS` regex.
    v1 replaced that constant with `hasControlChar()`, which scans by code unit
    the way links.ts does. The merge took #141's new `isDimSuffixFieldSegment`
    verbatim, so it called a name v1 had deleted. Swapped to the function; the
    two are byte-equivalent, tab exempted on both sides.

  · omp/markers.test.ts — #142's new cases bind `const row`, and this file has
    a module-level `row(name, i)` helper. main has no lint gate so it passed
    there; v1's oxlint calls it no-shadow, and it is right to. Renamed to
    `borderRow`, which is what v1's own version of the conflicted case already
    used.

The one textual conflict was the same assertion under two names. Took main's
"painted in one foreground end to end" — after #142 the rule reads a colour
CHANGE, so v1's older "no unstyled text to anchor against" no longer describes
what is being tested.
AltanS added a commit that referenced this pull request Aug 30, 2026
Cuts the merge of main's four fixes (05561ae) — #140, #141, #142, #121.
AltanS added a commit that referenced this pull request Aug 31, 2026
fix(omp): read the ghost by colour CHANGE, not by an unstyled draft
AltanS added a commit that referenced this pull request Aug 31, 2026
Brings #140, #141, #142 and #121 across. Two of them needed a hand on the way
in, because git merged the text and not the meaning:

  · codex/markers.ts — #141 was written against main's `CONTROL_CHARS` regex.
    v1 replaced that constant with `hasControlChar()`, which scans by code unit
    the way links.ts does. The merge took #141's new `isDimSuffixFieldSegment`
    verbatim, so it called a name v1 had deleted. Swapped to the function; the
    two are byte-equivalent, tab exempted on both sides.

  · omp/markers.test.ts — #142's new cases bind `const row`, and this file has
    a module-level `row(name, i)` helper. main has no lint gate so it passed
    there; v1's oxlint calls it no-shadow, and it is right to. Renamed to
    `borderRow`, which is what v1's own version of the conflicted case already
    used.

The one textual conflict was the same assertion under two names. Took main's
"painted in one foreground end to end" — after #142 the rule reads a colour
CHANGE, so v1's older "no unstyled text to anchor against" no longer describes
what is being tested.
AltanS added a commit that referenced this pull request Aug 31, 2026
Cuts the merge of main's four fixes (b5a38b1) — #140, #141, #142, #121.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants