docs: require a repro bundle with every fix - #580
Open
FelipeDefensor wants to merge 5 commits into
Open
Conversation
Human verification of AI-written fixes is the delivery bottleneck, and most of that time goes to scenario setup — clone the PR, launch TiLiA, create a timeline, create components — before any judging can start. Require every fix to ship a .tla fixture, a copy-paste launch command, and three lines of acceptance criteria, so reviewing costs one paste. `tilia` already accepts a .tla path positionally (boot.setup_parser), and a fixture with an empty media_path plus a "media length" in media_metadata opens with no prompt and no error, so this needs no code change. Two guardrails matter more than the convenience: - The fixture must reproduce the bug on the base branch. Fix and fixture are usually written by the same author, so a fixture that only ever shows the fixed state can encode the same wrong mental model as the fix and prove nothing. - The .tla is always deleted before merge. The file format changes and committed fixtures go stale silently; the durable artifact is the generator (CLI script or pytest), promoted into the suite as a regression test when the scenario is worth keeping. Bugs needing real media (playback, audiowave, video, PDF) can't ship a portable fixture — ask the user for a file instead of shipping one that can't reproduce the problem.
Three corrections found while building a bundle for PR #450: - All paths handed to TiLiA must be absolute. dirs.setup_dirs chdirs into the tilia package outside ENVIRONMENT=prod, and boot() parses its arguments before that chdir -- so `tilia repro/x.tla` passes argparse validation and then fails to open, and a CLI `save repro/x.tla` writes inside tilia/ silently. - The launch command has to go through the project environment. A bare `tilia` resolves to nothing unless .venv is active; use `uv run tilia "$PWD/repro/<issue>.tla"` from the repo root, which is both absolute and portable. - `gh pr checkout <N>` does not belong in a copy-paste launch line. It aborts when the head branch is checked out in another worktree, its fast-forward fails once the local branch has diverged, and --force discards the reviewer's local edits. It is one-time setup plus a warning, not part of the repeatable command.
requires-python allows <3.14, so a fresh `uv run` resolves to 3.13 -- the version the Project section already calls flaky under PySide. The documented command should not hand reviewers that environment.
The guardrail said the fixture must reproduce the bug on the base branch, but left the reviewer to get there by stashing the fix. The stash stack is shared across every worktree of a repository, and agent sessions run in parallel worktrees here, so a pop can restore someone else's work. Make it a fourth bundle item instead: a detached worktree at the base ref pointed at the same fixture file. It touches no shared state, leaves the reviewer's checkout alone, and gives them the failure to calibrate the acceptance criteria against.
…rove The bundle previously shipped a `.tla` (and its generator) as tracked files that had to be deleted before merge. That is a derived artifact kept honest by a promise: easy to forget, noisy in the diff, and it leaves an approved commit that must not merge. Generate into a gitignored `.repro/` instead. The scenario travels as CLI commands in the PR description, where it doubles as a readable statement of what state the reviewer is looking at, and nothing repro-specific enters history. The durable artifact stays what it always was: the regression test, written the way tests are normally written, with no coupling to the repro path. Also record the limit of the technique. The fix, the regression test and the scenario all come from one author with one mental model, so a before/after pair proves the scenario changed behavior, not that the scenario is the reported bug. A reviewer building the setup by hand was running an independent check; the bundle removes it while making the review feel more thorough. Deriving the scenario from the issue text and naming the uncovered cases is the mitigation. Rewrite the rationale accordingly: the argument for the bundle is that agents cannot drive the GUI and so cannot verify their own fixes, and that scripting the setup is now free for the author. Neither claim needs setup time to dominate review time.
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.
Why
Human verification of AI-written fixes is our delivery bottleneck, and most of that time is scenario setup — clone the PR, launch TiLiA, create a timeline, create components — before any judging can start.
This adds a
## Delivering a fix for human testingsection to CLAUDE.md requiring every fix to ship a repro bundle: a.tlafixture, a copy-paste launch command, and three lines of acceptance criteria.No code change needed
The mechanics already exist:
tilia /abs/path/to/file.tlaworks today — positional argument inboot.setup_parser."media_path": ""plus a"media length"inmedia_metadataopens with no prompt and no error (App._setup_file_mediareturns early on an empty path), so fixtures need no bundled media.script <path>+save <path>.tlafor generating fixtures reproducibly.Two guardrails
These matter more than the convenience:
.tlais always deleted before merge. The file format changes and committed fixtures go stale silently. The durable artifact is the generator (CLI script or pytest), promoted into the suite as a regression test when the scenario is worth keeping.Making the commands actually runnable
Building a bundle for #450 turned up three ways the original guidance produced commands that don't run. All three are now spelled out in the section:
ENVIRONMENT=prod,dirs.setup_dirschdirs into thetiliapackage, andboot()parses its arguments before that chdir. Sotilia repro/x.tlapasses argparse validation and then fails with "File not found", and a CLIsave repro/x.tlawrites insidetilia/without saying so.tiliaresolves to nothing unless.venvis active. The recommended form isuv run tilia "$PWD/repro/<issue>.tla"from the repo root — absolute after expansion, and portable without hard-coding anyone's checkout.gh pr checkout <N>does not belong in the copy-paste line. It aborts when the head branch is checked out in another worktree, its fast-forward fails once the local branch has diverged, and--forceresets the branch and discards the reviewer's local edits — which they may have made deliberately (e.g. to reach a code path the fixture can't trigger). It is one-time setup plus a warning, not part of the repeatable command.Known limitation
CLI coverage is uneven, so the CLI-script route doesn't fit every kind:
timelines addcovers hierarchy, marker, beat, score, range and audiowave — not harmony, pdf or slider.componentsregisters only thebeatsubparser, so it is not a general way to populate a timeline.timelines importis usually the better route when the fixture needs components: marker and hierarchy (CSV, by-time or by-measure), beat (CSV), score (MusicXML).Anything outside those lists falls back to a throwaway pytest that builds state via
commands.execute(...)and saves viacommands.execute("file.save", ...). A separate PR widening CLI coverage is in progress.Bugs needing real media (playback, audiowave, video, PDF) can't ship a portable fixture at all — the guideline says to ask for a file rather than ship one that can't reproduce the problem. YouTube-backed bugs are the exception: the URL lives in
media_path, so the fixture stays portable even though it isn't media-free.