feat: benchmaxxing cues preview CLI (closes #126)

[amarzullo24]

Summary

• Adds a cues preview --lane {image,text} --cue NAME --out DIR [--case PATH] [--case-id ID] [--image-root PATH] subcommand that renders a clean/contaminated cue twin to disk for eyeballing.
• Image lane writes clean.png / contaminated.png on a sample (bundled) or manifest-loaded image. Text lane prints the clean vs contaminated payload side by side in the terminal and writes preview.md.
• Falls back to a small bundled sample (a synthetic gray image, a three-option MCQ) when --case is omitted, so any cue can be checked with no data on disk.

Closes #126.

Test plan

• ruff check . passes
• pytest -q — 597 passed, 6 skipped (unrelated, pre-existing)
• Manually ran the literal command from the issue for both lanes (--lane image --cue watermark, --lane text --cue option_order) and confirmed the PNG pair / side-by-side output / preview.md

[maximinl]

Looks good: clean CLI wiring, sensible bundled-sample fallback, and solid coverage for both lanes plus the error paths.

One small optional follow-up: cli.main(["cues"]) currently exits 0 after printing usage. Returning non-zero (or routing through argparse's usual missing-subcommand behavior) would make bare cues fail the same way as other incomplete invocations. Not blocking.

[amarzullo24]

Good catch, thanks — fixed in 6c3c3f8: cues's subparsers are now required=True, so benchmaxxing cues with no subcommand fails through argparse's usual missing-subcommand path (SystemExit(2), "the following arguments are required: command"), same as any other incomplete invocation in the CLI (e.g. datasets stats with no manifest path). Dropped the now-dead _cmd_cues dispatcher and updated the test accordingly.

[sebasmos]

Second review to go with @maximinl's approval. Ran it off the branch: 15 tests pass, ruff clean, and the module is nicely self-contained — the bundled-sample fallback (flat gray image / three-option MCQ) means a cue can be eyeballed with no data on disk, which is a genuinely handy dev affordance. Scope is right: it renders twins to disk for inspection and touches nothing in the results path. The image extra being gated to the image lane only (text lane needs nothing beyond core) is the correct dependency split. LGTM.

[sebasmos]

Read the diff, the CLI wiring is clean, defaults to a bundled sample so it needs no data, and it is tested. A real reviewer-ergonomics win. Please rebase for the mergeable recompute; then good to go. Heads-up: #239 and #251 also touch cli.py, so whichever lands first will force a small rebase here.

[Agastya191]

Nice, self-contained tool: the bundled-sample fallback in preview.py lets any cue be eyeballed with no data on disk, the image/text dependency split is right (only the image lane pulls PIL), and the tests cover both lanes plus the error and bare-cues paths well. One thing worth tightening is format_side_by_side: it treats every payload field as a single physical line no wider than width, padding with ljust and pairing the two columns with zip. On a realistic case that breaks down: an option longer than 42 chars pushes the | divider past the column so the right side goes ragged, and a multi-line report (what the demographic_hint and imaging cases carry) puts an embedded newline mid-cell, so the clean cell's second line collides with the contaminated column and the two-column view scrambles. The written preview.md is unaffected since it wraps payloads in code fences, so this is only the terminal view, but that view is the point of the text lane. You have options here, but I'd split each cell on \n and wrap or truncate it to width before the zip, so rows stay paired and the columns stay aligned on non-trivial cases.

[sebasmos]

Agree with Agastya's comment on format_side_by_side, that's worth fixing since the side-by-side view is the whole point of this tool for the text lane. Once that's in, happy to merge.

[sebasmos]

Agree with Agastya's comment on format_side_by_side, that's worth fixing since the side-by-side view is the whole point of this tool for the text lane. Once that's in, happy to merge.

[sebasmos]

Still on the Jul 22 head, so the format_side_by_side fix is not in yet. One heads-up for when you do it: tests/test_cues_preview.py:68 asserts the contiguous substring [additional clinical detail], which any wrap-to-width will split, so that test needs updating alongside. I left it alone since the pairing is a design call for you.

[MohShahin]

Pushed a fix for the format_side_by_side line-wrapping issue Agastya flagged cells now split on embedded newlines and hard-wrap to width before pairing, so multi-line reports and long options stay aligned instead of scrambling the columns.

Updated test_cues_preview.py:68's assertion to check the unwrapped payload directly rather than a contiguous substring in the wrapped output, since that's what the test was actually trying to verify.

tests/test_cues_preview.py green (15/15).

[sebasmos]

Verified the format_side_by_side fix and it holds. _wrap_cell splits each payload line on embedded newlines then hard-wraps to width, and format_side_by_side pads per row before the zip, so both scenarios from the last round work: a 70-character option and a three-line report each keep the divider at column 42 on every row and keep the columns paired. Both lanes run end to end off the branch, and bare cues exits 2, so @maximinl's ask is met too.

My earlier note on tests/test_cues_preview.py:68 is withdrawn, and I was wrong to leave it open. Line 68 is a comment saying the opposite of what I claimed, and :69 asserts against preview._payload_lines(twin.contaminated), the unwrapped source, not the wrapped output. I checked directly: the contiguous substring is absent from side_by_side and present in _payload_lines. The test is correct as written, and you had already handled it in the same commit as the fix.

One thing left. The branch no longer merges cleanly: one add/add conflict in benchmaxxing/cli.py, where main added _select_hard_cases, _cmd_run and _cmd_report and this branch added _cmd_cues_preview. They are independent top-level functions, so keeping both hunks is the resolution. Resolved that way locally the suite is 1064 passed, 7 skipped, ruff clean, against 1049/7 on main. Rebase with that resolution and this is good to go.

[Agastya191]

This is in good shape now. _wrap_cell splitting each payload line on \n and then hard-wrapping to width, with the per-cell sub-line padding before the zip, fixes exactly what I raised, and the docstring's reasoning for avoiding textwrap so the [0] prefixes survive is the right call.

I ran it on the case I was worried about, a 58-character option plus a three-line report, and the left column stays exactly 42 wide on every row with the report lines staying in their own column. Full suite is green on the branch.

One thing for later rather than for this PR: _HANDLERS maps cues straight to _cmd_cues_preview instead of dispatching on args.cues_command the way _cmd_datasets does, so a second cues subcommand would silently land in preview. Worth a line whenever that second one arrives.

[sebasmos]

Approving, and apologising for the delay: my own stale review has been the only thing blocking this.

You delivered what I asked for in b9c5ae75 on 29 Jul. My CHANGES_REQUESTED from 28 Jul was never dismissed, and the three COMMENTED reviews I left afterwards do not clear it, so with @maximinl and @Agastya191 both approving, this PR has been sitting on a block from me that no longer had a reason behind it. That is my process error, not a gap in your work.

Re-verified at head, and by running it rather than only testing it:

• python -m benchmaxxing.cli cues preview --lane text --cue lexical_overlap --out ... prints a correctly aligned and padded clean-versus-contaminated table and writes the file. The format_side_by_side fix works on both scenarios.
• All 15 tests/test_cues_preview.py pass, including test_format_side_by_side_pads_uneven_columns.
• Argparse wiring survives a rebase; cues preview --help renders.

One thing you will need to do, because main has moved a long way since 22 Jul. GitHub reports this dirty. I rebased it in a worktree to check: exactly one file conflicts, benchmaxxing/cli.py, in a single hunk at around line 390 on current main, between main's _select_hard_cases and verify-bundle block and your _cmd_cues_preview. Keeping both hunks resolves it cleanly, with no reordering needed, since the two blocks are adjacent independent function definitions. The resolved file parses and the suite comes out at 1092 passed, 7 skipped.

So: rebase, keep both hunks in cli.py, and this is ready. Approving now so nothing waits on me again.