ci: make the review something you ask for, and tell it where the bugs are

[donislawdev]

Two things, one decision: the automatic review is withdrawn, and the briefing it uses
is rewritten to say where this repository's bugs actually come from.

The review becomes something you ask for

🔴 One run cost $2.92. Measured here: Opus 5, 13 turns, 6 minutes 42 seconds of model
time, on a pull request carrying a single Markdown file. The cost tracks how far the
reviewer explores, not how large the diff is - that run read the fake tkinter, the layout
suite and their docstrings to check one claim. At this project's rate that is a standing
monthly bill against a subscription with better uses.

So the trigger is inverted rather than tuned. The workflow now waits for @claude in a
comment and runs no other way - interactive mode, which is what dropping the prompt
input selects, so it answers the question asked instead of running a fixed task. It is
the emergency exit rather than the front door.

The turn count is deliberately not capped: when it is asked for, the deep read is the
whole point. timeout-minutes is the backstop. Still read-only, still the maintainer
only.

The guard gets stronger rather than weaker. It used to say "must not review on every
push"; it now says "must not run by itself at all" - no pull_request trigger, no
schedule - because half a trigger is the same bill.

The briefing says where the bugs are

The first version carried the conventions and nothing about failure modes, so a review
arrived knowing what it must not break and not where to look. The five classes it now
names are read off tests/test_mutation_registry.py rather than invented - lifecycle
races around targeting, view state drifting from fact, a warning that stops matching the
state it describes, a column registry drifting from the table it renders, and empty or
just-changed states.

🔴 First among them is the one no gate can see: a guard that cannot fail. An assertion
that would also pass over an empty set, a collector that returns nothing, a walk rooted
where a runner has no directory. The briefing gives the question to ask of every added
assertion - what input makes this red - and says outright that a test which would still
pass with the behaviour removed is a finding.

Kept short on purpose, since it is read on every run: the meta paragraph and the CI list
are cut back, and a what not to propose section is new, because the fastest way to
make a review worthless is to let it repeat the linter. No counts are written into the
prose - an unguarded number goes stale.

The briefing earned its keep before it was merged

The review ran on this branch and found a real defect in the briefing itself: one
paragraph described a stand-in that had since been rewritten. tests/fake_tk.py says so
in its own docstring, in the past tense. Verified against the source before changing
anything - it models eleven classes, records pack_info and grid_info, and
Treeview.column resolves a "#N" spec against the columns currently shown, which was
added precisely so the hidden-column header-tooltip bug would stop being invisible. So
layout and column mapping are covered; the two blind spots that are real are named
instead.

Steering a reviewer away from the best-covered part of the GUI would have been worse than
saying nothing, and this file becomes project memory for every later run. A second wrong
claim in the same file, found while checking the first, is fixed in the same commit.

🤖 Generated with Claude Code

[claude]

This describes the previous fake, not the one in the tree — and because this file becomes CLAUDE.md for every review run, it will steer future reviewers away from the areas that are in fact best covered.

• "one widget class" — tests/fake_tk.py defines seven with real behaviour: W, Text, Root, Notebook, Paned, Menu, Treeview. Its own docstring says this in the past tense:

  BeanNetworkTester/tests/fake_tk.py

  Lines 4 to 9 in 728729f

  	either. The previous fake was a single ``__getattr__`` no-op widget, so no test
  	could ever see a layout problem (``winfo_width`` was hard-coded to 400 and
  	``pack``/``grid`` did nothing). This one records geometry calls and models the
  	handful of widgets whose behaviour the app actually depends on - which is what
  	makes ``test_gui_layout`` / ``test_gui_state`` possible.

• "no geometry" — pack() records pack_info and even raises when before=/after= names a non-sibling; grid() records grid_info; Root.geometry() is modelled. Tests assert on it directly:

  BeanNetworkTester/tests/test_gui_layout.py

  Lines 163 to 171 in 728729f

  	bar = app.btn_start.master
  	assert bar.pack_info["side"] == "top"
  	bottom = bar.master
  	assert bottom.pack_info["side"] == "bottom" # anchored, not negotiable
  	assert bottom.pack_info.get("expand") in (None, False, 0)
  	assert app.log_box.master is app.log_wrap and app.log_wrap.master is bottom
  	# the notebook only gets what is left over
  	assert app.nb.master.pack_info["expand"] is True
  	""")

• "column mapping" — Treeview.column() resolves a "#N" spec against displayed_columns() exactly as Tk does, added specifically so the hidden-column header-tooltip bug would stop being invisible:

  BeanNetworkTester/tests/fake_tk.py

  Lines 557 to 579 in 728729f

  	def column(self, col, option=None, **kw):
  	"""Configure a column, or read one option back.
  	🔴 A "#N" spec is a DISPLAY position and is resolved against the columns
  	currently shown, exactly as Tk does (measured 2026-08-18). Without this
  	the fake could not tell the two rules apart, and the header-tooltip bug -
  	every column after a hidden one describing its neighbour - was invisible
  	to the whole suite while being obvious on screen.
  	"""
  	if option == "id" and str(col).startswith("#"):
  	try:
  	index = int(str(col).lstrip("#")) - 1
  	except ValueError:
  	return None
  	shown = self.displayed_columns()
  	return shown[index] if 0 <= index < len(shown) else None
  	entry = self.cols.setdefault(col, {})
  	if option is not None and not kw:
  	return entry.get(option)
  	entry.update(kw)
  	return None

  — which is the same bug this PR names as class 4 two paragraphs above.

Only "no style validation" holds: install() maps ttk.Style to a bare W, so style names and options are never checked and theme_use/map/layout are no-ops. The other true gap is that winfo_width/winfo_height/winfo_geometry return constants, so nothing measures real wrapping, clipping or overflow.

Suggested rewrite that keeps the intent and names the blind spot that is actually there:

• The fake tkinter models behaviour, not pixels. tests/fake_tk.py models the widgets
  the app depends on and records geometry and column calls, so layout and column mapping
  are covered (tests/test_gui_layout.py). What it cannot see needs a real renderer:
  ttk.Style is a bare widget, so style names and options are never validated and
  theme_use/map/layout are no-ops; and winfo_width/winfo_height/winfo_geometry
  are constants, so nothing measures real wrapping, clipping or overflow. Changes to
  gui/theme.py, or to anything that depends on measured text, deserve a closer read than
  their green suite suggests.

[donislawdev]

Correct, and verified against the source before changing anything: the docstring of tests/fake_tk.py says so itself, in the past tense. Fixed in 081cbcb, with the two blind spots that are real named instead - ttk.Style mapped to a bare widget, and winfo_width/height/geometry returning constants.

Steering a review away from the best-covered part of the GUI would have been worse than saying nothing, and this file becomes project memory for every later run, so it would have compounded. Thank you.

The same check turned up a second wrong claim in the header of that file, fixed in the same commit.