Repository navigation
fix(character): re-anchor the Format button on its custom element, not a tag that moved - #729
Merged
Merged
Conversation
…t a tag that moved
`--format-prompt` had degraded to a silent no-op on the migrated host: all three
entries of PROMPT_FORMAT_SELECTORS missed, `format_button_not_found` fired, the
prompt submitted as typed and the run exited 0 having spent image quota on a
prompt-engineering step that never happened.
Nothing was absent. Measured live on denon82 ($0 — navigation, one free
createEntity, DOM reads), with a control anchor so a flat zero could not be read
as absence:
control_check count=1 (prompt box resolved — the probe was in the editor)
personal_recommendations via i.google-symbols -> 0
personal_recommendations via i -> 0
span:text-is('Format') -> 0
the button itself, visible the whole time -> 1
<flow-format-prompt-button>
<button aria-label="Formatar">
<mat-icon class="... google-symbols ...">personal_recommendations</mat-icon>
Two independent misses, either one fatal:
a. Angular renders the ligature in <mat-icon>, not <i>. The ligature never
changed and mat-icon even carries the google-symbols class, so the class
was never the problem — the `i` tag was. This is the same carrier split
already fixed for add_2 / arrow_drop_down / accessibility_new in #703;
this constant was simply not swept with them, and nothing proves it was
the last one that was missed.
b. The button now has an aria-label, but Flow localises it ("Formatar" on a
pt account), as it does the <span>. The EN text fallback is deleted rather
than translated — display labels are banned as anchors (AGENTS.md).
The cascade now leads with the custom element <flow-format-prompt-button>: a
component boundary rather than a layout accident, unique among the six composer
buttons, and the strongest anchor class this frontend offers.
Two unit tests pin the contract that was missing: the cascade must carry BOTH
ligature carriers, and it must carry no display-label anchor. Both were watched
red against the old constant before the fix.
Verified by a live run, not by relaxing the assertion:
tests/e2e/test_character_create_e2e.py::test_character_create_format_prompt_clicks_format_button
1 passed in 219.90s — denon82, real generation, zero credits
Not measured: the labs frontend. No unmoved account exists here, so the two <i>
entries ride on the 2026-07-27 capture alone and were not re-verified today.
Closes #727
Nine-dimension council on 7c1d4c6 returned 4 GREEN / 5 YELLOW. Every finding below was raised by a reviewer, not by the author. D1/D14 - the retained 2026-07-27 comment still said, present tense, that the EN `span:text-is('Format')` fallback "has to be" the structural anchor, describing an entry the same commit deletes. The block also told the spike doc's story a second time, in the copy that will drift. Cut to the two DOM captures (the evidence) plus four lines, and a link to the finding. D4/D6 - `test_clicks_ligature_selector_first` kept its name and docstring while only its assertion moved to the custom element; the e2e's failure message still told a future drift victim the cascade is "anchored on the personal_recommendations ligature". A label that lies outlives the bug it describes. Renamed, reworded, and the e2e message now names the probe to run. Assertions themselves untouched - only the string a failure prints. D4 - the fall-through test drove only `PROMPT_FORMAT_SELECTORS[-1]`, so the new `<mat-icon>` entry at index 1 was never exercised. Parametrized over the whole cascade. D6 - `is_visible(timeout=1000)` is inert: Playwright documents the argument as ignored, and the call answers from the current DOM without waiting. Dropped, with the reason, so nobody re-derives "a 4-entry cascade costs 4s before the submit". D9 - the CHANGELOG called the capture "DOM read only" while the method it cites does a free `createEntity`, which is a write. Two files, one method, disagreeing. D5 - `flow-locale-leak-icon-ligatures.md` still prescribed the `<i>`-only pattern that caused this: #703 fixed three constants for the carrier split and never updated the memory that teaches it, so the fourth was written the old way on purpose. Corrected, and the split promoted to its own entry. D13/D2 - the spike put its guard in FRONT of its probe: a control miss raised before the dump, the screenshots and the report, deleting the evidence that would correct it. That is the #701 failure this file cites in its own docstring. Both controls now raise from a `finally` that has already written everything. A second control was missing entirely - nothing checked that the typed text landed, so a click intercepted by a consent overlay would have recorded a false absence indistinguishable from a real measurement. The scratch entity is deleted again, and the output path is root-anchored: `.gitignore`'s entry is slash-anchored, so running the spike from `scripts/dev/` wrote an UNIGNORED capture carrying the profile name, live URL, entity ids and authenticated outer_html. D14 - the occlusion probe measured the prompt box, not the button under investigation, and the spike doc filed that under "confirmed, not assumed". Deleted; the live e2e click landing is stronger evidence anyway. Re-verified live on denon82 after the rewrite ($0, 14s, clean exit): control_check count=1 typed_control chars=47 flow-format-prompt-button button -> 1 button:has(mat-icon:text-is('personal_recommendations')) -> 1 button:has(i.google-symbols:text-is(...)) -> 0 button:has(i:text-is(...)) -> 0 Declined, with reason: dropping `button:has(i.google-symbols:...)` as redundant with the broader `button:has(i:...)`. The superset argument is correct, but those are the labs anchors and no unmoved account exists here to re-verify against - narrowing an untested surface to save one line is the wrong trade. Refs #727, #730, #731
`FlowApiClient._checkout_page()` is documented as blocking forever - "Waits indefinitely if the pool is exhausted (no Pages available). Callers that need a deadline must wrap the call themselves" (api/client.py) - over an `asyncio.Queue(maxsize=n)` that holds ONE page by default. A script that takes that page and never returns it has not leaked a resource; it has armed a deadlock for the next caller that needs one. Nothing usually does, which is why this sat latent in seven scripts. Then spike_character_prompt_format.py added a `client.delete_characters(...)` cleanup call in its `finally`, obeying skills/spike/SKILL.md's "delete anything the spike created". Every `client.<verb>()` reaches Flow through `_post_json`, which checks out a page of its own - so the cleanup blocked on a queue the script itself had emptied. It failed in the worst available shape. All work completed: the capture and all three screenshots were on disk. No error, no traceback, no output. The process simply never exited, holding the denon82 profile lease. `contextlib.suppress` cannot rescue it, because a block is not an exception. And a wedged process holding a lease is exactly what sends an operator to the process list - the sequence that cost nine browsers belonging to a running e2e suite the same day, and the other half of what test_spike_profile_lease.py exists to prevent. Measured before fixing: 7 of 24 scripts under scripts/dev/ never check in. Of those, only spike_character_prompt_format.py can deadlock TODAY - the other six make no client call after taking the page. They are fixed anyway: "latent" is precisely what the seventh was until this afternoon. The 17 scripts that already pair the calls are the reference, and `_post_json`'s own attempt() is the canonical shape - checkout, try, finally checkin. tests/scripts/test_spike_page_pool.py pins the invariant offline and text-level, in milliseconds, with no browser, mirroring test_spike_profile_lease.py including its scan-still-resolves guard so a renamed pool API cannot empty the check silently. Watched failing against all seven scripts by name before the fix. The six sibling diffs are almost entirely re-indentation - `git diff -w` collapses each to its added try/finally. Every script re-verified: ast parses, --help runs. Fix verified by a run, not by inspection: the spike now exits 0 in 14 seconds and logs `deleted_scratch_entity`, where it previously hung indefinitely.
Phase 0 for the question #727 deliberately left open: should a missed Format button be loud? That cannot be answered until we know what a HIT looks like. `format_character_prompt` clicks, waits ~500ms of jitter, logs `ui_automation.prompt_formatted`, and returns - and `_send_prompt` submits immediately after. So the event proves a CLICK, never a REWRITE ([[playwright-click-no-downstream-event-signature]]). If Flow reshapes the prompt via a server round trip, 500ms may be short, and the submit could carry the text the user typed while the rewritten text was still in flight: the flag doing nothing, behind a green exit code AND a green e2e, which asserts the click event. This script measures it rather than reasoning about it: seed a deliberately terse prompt, confirm it landed, click Format through the SHIPPED cascade (so every run also re-tests the #727 anchor), then poll the box every 250ms for 20s recording when the text changes, while capturing every request the page makes. A no-change result is reported WITH its timeline, so "unchanged after 20s" is a positive observation with a shape rather than a wait that expired. Never submits: no image, no video, no credit. Not measured, and said so in the docstring: whether the rewrite call itself draws on server-side quota. Carries the discipline the council extracted this afternoon - lease via FlowApiClient, page returned before any client verb, controls that raise only from a finally that has already written the evidence, scratch entity deleted, root-anchored output path. Refs #727
Owner
Author
|
…it 0.5s Measured on denon82, $0, never submits. #727 fixed the anchor; this measures the step after it, which the council (D6) flagged as unevidenced: the #727 spike captured no network, so nothing established the click reached a backend, let alone that its answer arrived before the submit. baseline 'girl, red hair, sad' 19 chars format_clicked flow-format-prompt-button button box_text t=0.011 chars=19 'girl, red hair, sad' <- unchanged box_text t=5.355 chars=651 'Medium studio shot of a young girl with...' verdict changed=True changed_at_s=5.355 within_current_wait=False t=1.191 POST .../batchexecute?rpcids=eAenfb <- server round trip gflow waits _jitter_ms(500) and submits on the next line, so --format-prompt discards the reshaped prompt on every run - including runs where the button is found, clicked, and ui_automation.prompt_formatted is emitted. The flag has never worked on this host: before #727 it missed the button, after #727 it throws away the answer. The e2e does not catch it and is not wrong to be green - it asserts the click, because when it was written the click was the only observable. Fixable: the rewrite is one discrete DOM swap, so the settle signal is a condition to poll (box text differs from what we inserted), not a duration to sleep - and comparing against OUR OWN string is locale-invariant by construction. Not measured, and recorded as such: how variable 5.4s is (n=1), what the box does when eAenfb errors or rate-limits, the labs frontend at all, whether the rewrite draws quota, and idempotency of a second click. Refs #727
Owner
Author
|
| Persona | Score | Verdict | Headline |
|---|---|---|---|
| P1 Architect | 8 | GO | Fits the existing settle-gate idiom; invent nothing |
| P2 Security | 7 | GO* | No auth surface; the fingerprint moves toward human — but do not log the rewritten text |
| P3 Performance | 6 | CAUTION | Design is right; the timeout must not be reverse-engineered from n=1 |
| P4 CLI/MCP UX | 8 | GO | No MCP surface to break; hazards are doc/telemetry edits, not design risk |
| P5 Devil's Advocate | 7 | CAUTION | Withdrew its own duplication objection; wants #731 landed first |
Average 7.2, +1 for unanimous agreement on the approach. The two CAUTIONs are conditions on the number and the ordering, not objections to the design.
The one measurement that decides the mechanism
The
eAenfbresponse arrived at t=1.191s. The DOM settled at t=5.355s.
P1 and P3 independently rejected gating on page.expect_response(rpcids=eAenfb), and P3 gave the killer reason: the response precedes the DOM by 4.2s, so a wire-level gate re-creates the same early submit. It also hardcodes an undocumented batchexecute id into ui_automation, against the structural-anchor rule, on a host whose labs twin was never measured. Keep the rpcid as a logged diagnostic; never as the gate.
Conditions carried into the plan
Unanimous (3+ personas independently):
- Length delta, not
!=. Slate/ProseMirror re-renders and normalises whitespace; raw inequality against the inserted string false-positives and firesprompt_formattedon a non-rewrite — today's bug with better telemetry. Observed signal was 19 → 651 chars. - Match the existing settle-gate idiom —
time.monotonic()deadline +page.wait_for_timeout(poll), as_locate_body_prompt_boxand five siblings already do. No new helper, nowait_for_function(locators overevaluate), noexpect()(appears nowhere in this file). - Stay best-effort. No new exit code. Promoting the miss to an exception lands it in
EXIT_CODE_MAP, the MCP schema and the docs at once — and the body-prompt call site runs after the face image spent quota, so failing there aborts a saga mid-spend.
P1 — Architect:
4. The baseline is a parameter, not instance state. Both callers already hold the string they inserted. Stashing it on self gives a page-reused transport hidden temporal coupling across generations.
5. Keep -> bool; change its meaning, do not widen it. Both callers discard it, so enriching it builds a channel with no consumer.
6. Module constants, no env var (_BODY_SLOT_MOUNT_TIMEOUT_S precedent). No config nobody sets.
P2 — Security:
7. Never log the rewritten text, and never a truncated excerpt. Flow elaborates a terse person description into a detailed physical one — the output is more PII-dense than the input, and GFLOW_CLI_HISTORY_PROMPTS governs the DB, not structlog. Log prompt_len_before / prompt_len_after + the 8-char _prompt_hash_stable, matching same-file precedent. A 70-char head is exactly where the physical description lands.
8. Jitter the poll interval — a fixed 250ms cadence is itself a deterministic signature, and _jitter_ms exists in this module for that reason.
9. Monotonic deadline inside the existing try/except, so an eAenfb hang cannot leave the page held.
P4 — CLI/MCP UX:
10. Redefine prompt_formatted to mean the observed rewrite (it already claims to); add format_button_clicked (old semantics) and format_not_observed (timeout). CHANGELOG it as a semantics change and fix docs/LIVE_VERIFICATION_v0.45.0.md:106, which asserts the old meaning.
11. The --help is currently false — the button is found and the prompt is still submitted as typed. Rewrite it to name both failure modes and disclose the ~5-15s cost.
12. Expose prompt_formatted: false in --json so agents detect the degrade without parsing stderr.
13. No MCP surface exists (mcp/tools.py, worker/codec.py, docs/MCP.md all clean; character create is _MCP_EXEMPT for GAP #691). The exemption's reason still holds and this strengthens it — a longer inline call is direct input to the worker-enqueue decision (#481). Record that in the #691 port note.
14. Strengthen the e2e in the same PR, or it re-certifies the click.
Unresolved conflict — surfaced, not suppressed
The timeout value. P1 says ~15s (labs is unmeasured; there a no-rewrite pays the full budget before submitting). P3 says ~30s (costs are asymmetric — too long is bounded latency on an opt-in flag, too short silently reintroduces the exact bug, invisibly). P4 says ~20s.
Resolution: take the generous end, and make the number stop mattering. P5's framing is the strongest: the fallback on expiry is today's behaviour, so a generous cap means n=1 is no longer load-bearing. P1's labs concern is real but bounded, and only on an opt-in flag. ~30s cap, 250ms jittered poll, and log elapsed_ms on every success so the distribution accrues in production rather than being demanded before shipping.
P3 also notes the evidence is weaker than n=1: the 250ms cadence bounds the true settle only to (5.105, 5.355].
Sequencing
P5 argues #731 first: docs/CHARACTER.md prints stale selectors marked VERIFIED, mirrored to the published website. A false claim marked verified actively misleads; this proposal fixes a flag with four months of zero reports. Then this change. Then #730 as its own predicted sweep — not another hand-fix.
Cheapest experiment, if a tighter number is ever wanted
scripts/dev/spike_format_prompt_effect.py already does one run at $0, never submitting. Loop it: 5 prompts of varying length, one already-formatted (idempotency), one under DevTools network throttling (the error/empty path). ~15 minutes, no quota. Labs remains a named blocker — no unmoved account exists here.
…ionale copied six times Two review passes on `git diff 7c1d4c6..HEAD`, run because this repo has a documented pattern of the fix getting less scrutiny than the original - and because this session had already reproduced it once today. Regression pass (GREEN, one must-fix): the try/finally re-indent pushed two lines in capture_image_add_media_dom.py past 100 chars. Invisible to /gflow:check, whose lint scope is `src tests` - `scripts/` has never been in it. Reflowed; all eight files this PR touches now pass `ruff check --select E501` (the 64 remaining in scripts/dev/ are pre-existing in files this PR never opened). That pass also proved the six re-indents semantics-preserving rather than reading them: an AST transformer spliced out every Try whose finalbody is exactly a _checkin_page call, then compared ast.dump before/after. Four files identical; two differ only in interior whitespace of JS string constants, with no template literals, so JS behaviour is unchanged. It also replayed the new guard against 7c1d4c6 and got offenders=7 - the guard fails pre-fix, so it is not vacuous. YAGNI pass (GREEN, three cuts): - the 4-line rationale for the checkin was copy-pasted into all six scripts. That is the "do not copy the table around" failure, committed inside the fix for a bug caused by duplication. Collapsed to one line pointing at the test that owns the explanation. - test_spike_page_pool.py re-told the nine-browsers incident that is the entire subject of test_spike_profile_lease.py's docstring one file over. Now a pointer. - the spike doc pinned a literal count of single-carrier selectors, which is a number that rots. Deferred to the memory entry and #730. Verified: ruff check/format src tests clean, 295 passed (tests/scripts + test_ui_character_editor), doc links resolve, 49 memory files all cited, and all six re-indented scripts still import and --help. Known and out of scope: spike_image_upscale_drive.py has three bare _checkin_page calls with no try/finally, so an exception between checkout and them still leaks. The new guard is a text-presence check and passes it. Pre-existing.
…d of a third copy ponytail-review pass over the full PR diff, run because the previous passes were scoped to a delta and the last commit had been reviewed by nobody. The finding is mine and it is the same shape as the bug this PR fixes. The character-editor ready selector was written out THREE times: once canonically as UiAutomationTransport._CHARACTER_EDITOR_READY_SELECTOR, and copied verbatim into two spike scripts. D13 flagged the first copy as a nice-to-have; I did not act on it, then added a second copy in the new spike. A probe that asserts a different anchor than the code it is probing measures the copy, not the code - which is exactly how a selector and its documentation drift apart, and #727 is what that costs. Both scripts already imported the class, so this needs no new import. Also, in spike_format_prompt_effect.py: two monotonic bases (t_zero for requests, t_click for polls) forced `r["t"] >= t_click - t_zero` in the verdict line. One clock now, with click_at recorded in the report - the request and poll timelines become directly comparable, which matters because the gap between them (response at 1.191s, DOM at 5.355s) IS the finding. And `timeline[-1] if timeline else None` was dead: the poll loop always runs at least once. net: -8 lines. Verified: ruff clean on both, and both modules import with identical resolved selectors.
/code-review at high effort, run on the full branch - the first review to see anything past 7c1d4c6. Seven findings; these are the four that are claims rather than behaviour, and a claim is what outlives the code. CHANGELOG (finding 3) sat under "### Fixed" describing the bug as "the image quota spent on a run whose requested prompt-engineering step never happened", which implies that step now happens. It does not. The entry is scoped to what was measured - the anchor - and a new "### Known issues" section states plainly that --format-prompt still submits the prompt as typed. --help (finding 2) promised Flow would rewrite the prompt and named "button not found" as the only failure. That is the one case it gets wrong: the prompt is submitted as typed even when the button IS found and clicked. Now names the real limitation and keeps the best-effort promise it can actually honour. The framing is deliberate and comes from a peer session reviewing the same class of defect. The bug is NOT that the wait is too short - that reads as tuning. It is that gflow reports success without observing it: prompt_formatted is emitted on the click, so the absence of a completion inside a window we chose is recorded as a completion. Same class as a 20s selector timeout read as "the feature is absent" (#701) and a bundle photographed after teardown read as "the DOM was gone" (#722). This one is the worst of the three because it fails toward SUCCESS - a premature green looks exactly like the feature working, and nobody investigates a pass. test_cascade_carries_no_display_label_anchor (finding 6) did not enforce the rule its own docstring stated. It banned the byte sequence 'Format' in single quotes, so it passed for "Format", for [aria-label*="Format"], and for 'Formatar' - the exact string the spike observed on the pt account. Banning spellings loses to translation by construction. Replaced with a whitelist: every text argument in the cascade must BE a known ligature. Proven non-vacuous - accepts all four shipped selectors, rejects all four bad spellings including 'Formatar'. The is_visible comment (finding 7) claimed "a miss is one round trip". True for a miss, false for a click timeout: entries 0 and 1 resolve to the SAME element on migrated, 2 and 3 to the same element on labs, so a timeout is retried on an identical element and the 5s budget is paid twice. Comment now states the real worst case. Not de-duplicating the resolved elements here - that belongs with the observed-rewrite rewrite, not in front of it. NOT fixed here, recorded instead: - finding 1/5, the ~5.4s wait itself. /gflow:predict returned GO with conditions (8.2/10); it is a behaviour change on a paid path and gets its own PR. - finding 4, format_character_prompt resolves .first page-globally while _submit_body_prompt deliberately targets the LAST mounted box. On a two-box cohort the format click lands on the PORTRAIT composer. Real, and I have no two-box cohort to verify a fix against - naming it rather than blind-fixing it. Verified: ruff + format clean, pyright 0 errors, 295 passed, doc links, hygiene (1022 files), website mirror, 49 memory files, PII scan, and the corrected --help renders.
Fixes the second half of #727, and the half that actually made the flag useless. The anchor fix made the button findable; this makes the click mean something. Flow rewrites SERVER-SIDE. Measured 2026-09-07 on denon82: the click fires a batchexecute round trip and the reshaped text reaches the composer at ~5.4s. The old code waited _jitter_ms(500), logged ui_automation.prompt_formatted and returned True; _send_prompt submits on the very next line. So --format-prompt shipped the prompt the user typed and threw the rewrite away, on every run, with a success event in the log and exit 0. THE DEFECT WAS NOT THE DURATION. It was reporting a success we had not observed: the absence of a completion inside a window we chose, recorded as a completion. Same class as a 20s selector timeout read as "the feature is absent" (#701) and a bundle captured after teardown read as "the DOM was gone" (#722) - a measurement that could not be taken, recorded as a measurement. This one is the worst of the three because it fails toward SUCCESS: a premature green looks exactly like the feature working, so nobody investigates it. Framing owed to the peer session that named the class while reviewing the same shape in its own judge. format_character_prompt now polls the bound composer until its text actually changes, and True means observed-rewrite, not button-pressed. Design per /gflow:predict (GO with conditions, 8.2/10, five personas): - Gate on the DOM, NOT on the wire. eAenfb's response arrives 4.2s BEFORE the text settles, so awaiting it reproduces the same early submit - and an undocumented batchexecute rpcid is not an anchor worth depending on. Comparing against the string we inserted never reads a display label, so it is locale-invariant by construction. - Length delta, not `!=`: Slate/ProseMirror normalise whitespace and bare inequality would fire on churn, re-reporting the false success with better telemetry. Normalisation moves a handful of chars; the observed rewrite moved 632 (19 -> 651). Threshold 16. - Baseline is a PARAMETER, not instance state - holding it on self would couple it across generations on a reused page. Both callers already had the string. - Existing settle-gate idiom (monotonic deadline + jittered poll), not a new helper; module constants, no env var nobody would set. - Still best-effort, still `-> bool`: promoting a miss to an exception would land in EXIT_CODE_MAP, the MCP schema and the docs at once, and the body-prompt call site runs AFTER the face image already spent quota, so failing there aborts a saga mid-spend. - 30s budget, generous on purpose. Costs are asymmetric: too long is bounded latency on an opt-in flag whose expiry falls back to exactly the old behaviour; too short silently reintroduces the bug. elapsed_ms is logged on every success so a real distribution accrues instead of being fitted to n=1. - Lengths and a stable hash only, never the text. Flow ELABORATES a terse description into a detailed physical one, so the rewrite is more PII-dense than the input, and structlog is not governed by GFLOW_CLI_HISTORY_PROMPTS. Pinned by test_never_logs_the_prompt_text. Also fixes /code-review finding 4, which no other reviewer caught: the button was resolved page-globally with .first while _locate_body_prompt_box documents "the LAST mounted box is the target - never the portrait's". On a two-box cohort the format click landed on the PORTRAIT composer, reshaping the wrong prompt or finding a disabled button and giving up. _locate_format_button now mirrors that existing convention instead of inventing a second one. And a disabled match `continue`s rather than `return False` - one stale anchor used to kill every selector behind it, including the one that would have worked. Events: prompt_formatted now means the observed rewrite (it always claimed to); format_button_clicked carries the old semantics; format_not_observed reports a timeout without claiming Flow failed - an unchanged box also covers Flow declining, erroring, or judging the prompt already formatted, none of which were provoked, so none are claimed. Five tests written first and watched failing: TypeError: format_character_prompt() got an unexpected keyword argument 'prompt_box' Then 912 passed in tests/api/transports/, pyright 0 errors, ruff clean. An autouse fixture shrinks the 30s budget for the offline suite - wait_for_timeout is mocked, so the loop would otherwise spin on wall-clock for a full minute across two never-lands cases. Still owed and not claimed: a live run proving the gate on real Flow.
Two defects, found by my own change overrunning the budget. 1. The budget was fitted to the old behaviour. A create drives up to two prompts and --format-prompt now waits for Flow's server-side rewrite on each, so the worst case grew by up to 60s on a run that already took ~220s. 300 -> 480. 2. A timeout HUNG instead of failing. subprocess.run(timeout=) kills the child then re-enters communicate() to drain the pipes, but a browser-driving child leaves Chrome grandchildren holding the inherited write handles, so the drain never returns. Observed: 32 minutes at near-zero CPU, indistinguishable from slow progress, until it was stopped by hand. An unbounded wait after a timeout defeats the timeout. My first fix for (2) bounded the drain. It stopped the hang and then threw the output away, so the very next failure reported a command and a duration and not one transport event - I had traded a hang for a blind failure. Redirecting to files removes the class instead: nothing blocks on a reader, and whatever the run logged before it died is on disk and readable afterwards, including on the timeout path. Five sibling e2e modules carry their own pipe-based _run_gflow. Named in the docstring rather than changed: only this one drives a browser long enough today for the difference to show, and I cannot exercise the timeout path in the others. The e2e assertions also now distinguish the two failure modes the gate created - format_button_not_found (the anchor drifted) vs format_not_observed (clicked, no rewrite) - and name the probe to run for each. prompt_formatted became load-bearing for free: it is emitted only on an observed rewrite now, so the assertion that passed for a year while the feature was broken finally means what it says.
The live e2e passed - 1 passed in 406.20s, denon82, asserting the OBSERVED
rewrite plus format_button_clicked and rejecting format_not_observed,
format_click_failed and format_button_not_found. So --format-prompt works end to
end, and every claim I wrote an hour ago saying it does not is now false.
I wrote those claims, in four places, and they are exactly the kind that survive
because nobody thinks to look: a CHANGELOG "Known issues" block, a --help string
users read, a spike doc title, and that doc's conclusion. Swept by grep, not by
recall.
- CHANGELOG: the Known-issues block is gone; the behaviour change is recorded
under Changed, including what prompt_formatted now means and the two new
events beside it.
- --help: no longer advertises a KNOWN LIMITATION that no longer exists. It now
names the real cost, because there is one.
- the spike doc keeps its title in the past tense with a RESOLVED banner. It is
a dated measurement, not a standing limitation, and deleting it would remove
the evidence that makes the fix defensible.
MEASURED COST OF THE FIX, which the docs now state instead of implying:
--format-prompt takes 406s against a 210s control on the same account and face
prompt. Roughly double, and only partly the ~4-5s rewrite - the rest is Flow
generating from a 608-character elaboration instead of the 102 characters the
user typed. That is not a regression. It is what the feature costs, and it was
free only while it was doing nothing. It is also why _CREATE_TIMEOUT_S moved from
300 to 480: the old budget was fitted to a flag that did not work.
Verified: ruff + format clean, pyright 0 errors, 319 passed across the CLI,
transport and scripts suites, doc links, hygiene, website mirror, memory round
trip.
Owner
Author
Live e2e green — both faults fixedTransport events from a direct run of the same path: Click → observed rewrite → submit, in that order. The inversion of those last two is the entire bug. Telemetry carries lengths and a stable hash and no prompt text, per the predict's security condition. The control, because a result without one is a coincidence with formatting
Generation on this account was never broken. The overrun tracks the flag, and only ~9s of it is the gate — the rest is Flow generating from a 608-character elaboration instead of 102 characters. 406s vs 210s, roughly double. That is not a regression. It is what the feature costs, and it was free only while it was doing nothing. It is also why the old 300s budget failed: that budget was fitted to a flag that did not work. Claims I had to retractAn hour ago I wrote, in four places, that this flag does not work. All four are now false and all four were mine — a CHANGELOG "Known issues" block, the The spike doc keeps its past-tense title under a RESOLVED banner rather than being deleted: it is a dated measurement, and it is the evidence that makes the fix defensible. Also fixed, found by the overrun
Five sibling e2e modules carry their own pipe-based |
Merged
6 tasks done
tranzmatt
pushed a commit
to tranzmatt/gflow-cli-unofficial
that referenced
this pull request
Sep 9, 2026
… the false VERIFIED Fixes ffroliva#731. docs/CHARACTER.md section 12 marked rows **VERIFIED** that stopped being true at ffroliva#703, and the file is mirrored to website/docs, so the false claims are published. A row marked verified that is not is worse than no row: it is what a reader trusts INSTEAD of reading the code. That is how ffroliva#727 got written - the carrier split was documented nowhere a selector author would look, and the table confidently said otherwise. Every row re-derived by reading the constant, not edited from memory. What that turned up, beyond what ffroliva#731 reported: - Model picker: the documented selector carried `aria-haspopup='menu'`, which appears in NONE of the four shipped entries. Not merely stale - it never matched the shipped tuple. - Generate / submit: SUBMIT_BUTTON_SELECTORS is single-carrier, and on the migrated host BOTH `<i>` entries miss. Only the third, `has-text('arrow_forward')`, fires - matching the mat-icon's text rather than its tag. Observed live 2026-09-07: `prompt_submitted via="button:has-text('arrow_forward')"`. It works by luck, not by design; noted in the row and tracked in ffroliva#730. - Prompt box: the row claimed PROMPT_INPUT_SELECTORS[0]. On the migrated host [0] misses and [1] `div[contenteditable="true"]` is what matches (live, same run). - Editor-ready anchor and body-mode chip: both gained mat-icon / custom-element alternatives at ffroliva#703 that the rows never picked up. The undated **VERIFIED** marker is replaced by a per-row date naming what the row was checked against - code, live run, or both - so a row cannot silently outlive its evidence again. The carrier split is stated at the top of the section, where someone writing a selector will actually meet it. Deliberately NOT included: the Format-button row and the docs/USAGE.md `--format-prompt` entry. Both describe behaviour that is still changing in ffroliva#729, and documenting a flag from this branch would describe the broken version. They ship with that PR. Also removed before commit: a [[ligature-carrier-differs-by-host]] wikilink I had added here. That memory file lands with ffroliva#729, not develop - a pointer to something that does not exist yet is the same defect this PR is fixing. Verified: website mirror regenerated and in sync, doc links resolve, PII scan clean, hygiene 1023 files. Three pre-existing dangling wikilinks remain in this file (private-store-only memories); left alone, out of scope.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Lifecycle
character createis an explicit_MCP_EXEMPTintests/mcp/test_cli_parity.py, so there is no MCP twin to driftspikerun before any claim —skills/spike/SKILL.md, finding indocs/superpowers/spikes/2026-09-07-character-format-button-anchor.mdpredictscenario+plancheckgreen, including the step 1b mirror sweep (all six axes below)character create; zero hits forformat_promptundersrc/gflow_cli/mcp/tests/e2e/test_character_create_e2e.py::test_character_create_format_prompt_clicks_format_button,1 passed in 219.90sondenon82, real generation, zero credits. This test was RED before the fix and is green by a live run — the assertion was never relaxed.Validation
test_cascade_covers_both_ligature_carriers—AssertionError: cascade must carry the migrated-host <mat-icon> carriertest_cascade_carries_no_display_label_anchor—AssertionError: "button:has(span:text-is('"'"'Format'"'"'))" anchors on a localised display labeltest_clicks_ligature_selector_first, updated to the new primary anchoruv run python scripts/ci/check_repo_hygiene.py— 1017 files, no violationsuv run python scripts/ci/check_doc_links.py— all links resolved across 29 filesuv run python scripts/ci/check_website_docs_pii.py— no private identifiers, 25 filesuv run python scripts/ci/generate_website_docs.py --check— mirror in syncuv run python scripts/ci/check_council_memory.py— 48 memory files, all citeduv run ruff check src tests/ruff format --check src testsuv run pyright src— 0 errorsuv run python -m pytest -q --cov=gflow_cli— 4009 passed, 7 skipped, 92% coverageStep 1b blast radius — six axes
Grepped
format-prompt/format_prompt/personal_recommendationsacrosssrc/gflow_cli/mcp/,docs/,website/docs/,README.md,KNOWN_ISSUES.md,skills/.src/gflow_cli/mcp/;character createexemptskills/,README.md,docs/INDEX.md, AGENTS.md command surfaceEXIT_CODE_MAPentryLIVE_VERIFICATION_v0.45.0.md,PROJECT_STATUS.md, the 2026-07-26 PLAN) that correctly describe what was true thenWhat this does NOT prove
<i>entries are carried forward on the 2026-07-27 capture alone. They were not re-verified today and are not asserted to still work.Open question, deliberately not acted on here
--format-promptstays best-effort, which is what its help text promises and what #727 says is correct. But that is why this went unnoticed for an unknown period: a paid run silently skipped the step the user asked for behind exit 0, and only an opt-in e2e (GFLOW_CLI_E2E_RUN_CHARACTER=1, not in the nightly canary) could catch it. Whether a missed button should be loud — or fatal under an explicit--format-prompt— is a real design change and belongs in its own issue.Contribution Checklist
developscripts/dev/_spike_out/