SWG-4A-10: griff swang dump - #192
Merged
Merged
Conversation
Tests only. `cli/tests/swang_dump.rs` states the whole contract for
`griff swang dump`; nothing under `cli/src/` changes, so the suite cannot
compile:
error[E0432]: unresolved import `griff_cli::swang_dump`
--> cli/tests/swang_dump.rs:29:16
error: could not compile `griff-cli` (test "swang_dump")
and the binary agrees, at runtime:
$ griff swang dump /dev/null
error: unrecognized subcommand 'dump'
exit: 2
The eight obligations, one test each (plus one for the seam's other half):
1. a Guitar Pro file dumps a canonical level-2 document;
2. a MIDI file does the same, through the same entry point;
3. stdout carries the document and no CLI chatter — asserted both as the
absence of specific noise and, more strongly, as equality with
`write_score` of the imported score;
4. an import warning is reported on stderr **and** kept in the exact text.
These are two surfaces, not one: stdout carries a canonical fact, stderr
a courtesy. Dropping a `loss` entry because a human already saw it would
undo what SWG-4A-05 established;
5. two runs of one file produce byte-identical stdout;
6. an unimportable file exits non-zero with an empty stdout;
7. a score outside the writer's domain yields no document at all — proved at
the library seam, because the importer sanitises its own output and no
file reaching `dump` is refused (ppqn 0 is rejected by the MIDI reader,
and a zero meter numerator is normalised to 4/4 before the writer sees
it). The composition returns the whole document or nothing, so there is
no partial value a caller could print;
8. no hidden normalization: the CLI's stdout is compared against
`write_score(import_score_auto(bytes))` rather than a hand-copied golden,
which is what makes it a transport test instead of a second formatter
agreeing with the first about something wrong.
Both fixtures come from encoders independent of the code under test: `midly`
for MIDI, and the `guitarpro` crate's own serializer for Guitar Pro. The
latter is a new dev-dependency of `griff-cli` only — the crate is already in
the tree through `griff-core`'s default `gp` feature, so this adds test-time
access rather than third-party code.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018vdRzztKXy8bA16tEzwgLE
`griff swang dump input.mid` / `input.gp5` imports through the existing adapters and prints the canonical Swang level-2 exact text. The command is a transport, not a second formatter. Everything it could get creative about was already decided by the census and implemented by SWG-4A-05, so the only real decision here is which stream each rendering reaches: - stdout gets the document from `exact::write_score`, verbatim and whole; - stderr gets one line per import warning, plus any diagnostic. `griff_cli::swang_dump::dump_score` builds both before either is written and returns them together. That is what makes "no partial document" structural rather than a discipline someone has to remember: a refusal produces no `DumpOutput` at all, so there is no half-formed value for `main` to print. It is also the seam the tests use to reach the refusal path, which the importer never produces on its own — the MIDI reader rejects ppqn 0 outright and normalises a zero meter numerator to 4/4 long before the writer sees it. The `loss` report deliberately appears on both surfaces. It is a canonical fact the exact text is obliged to carry (§2.8); the stderr line is a courtesy to whoever is watching the terminal. Dropping the block because a human already saw the warning would make the document depend on who was looking at it. Nothing in `griff-swang` or the importers changed — the diff is one new `griff-cli` module, one clap subcommand, one dispatch arm, and one `CliError` variant carrying `ExactWriteError`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018vdRzztKXy8bA16tEzwgLE
Tests only; no production line changes. Eleven mutations of the dump path were probed, one at a time, each against the whole `griff-cli` suite. Ten were caught. One survived: sorting the stderr warning lines. Nothing in the suite looked at their order — the matrix only counted them — so a rendering that alphabetised the loss report would have passed. `LossReport` appends and the exact walker compares positionally (§2.7). The stderr lines are a rendering of that vector, so they inherit its order rather than choosing one; that is now checked rather than assumed. The fixture has to be able to see the difference, so it is built to: eleven tracks, with invalid UTF-8 names on raw indices 2 and 10. "track 10" sorts before "track 2", so vector order and sorted order disagree — and the test asserts they disagree, which is what stops it from passing vacuously if the fixture is ever weakened. Both halves are checked: the `DumpOutput` list and the order those lines actually reach stderr in. Verified: the new witness fails under exactly the mutation that motivated it (`sort the warning lines` → CAUGHT by `the_warning_lines_keep_the_loss_report_order`), and a full re-run of all eleven probes now reports 11 CAUGHT, 0 SURVIVED, 0 NOT REBUILT. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018vdRzztKXy8bA16tEzwgLE
Marks SWG-4A-10 done in the task index and its own section, records the acceptance actually met, and moves the "next" marker off the writer lane. The writer lane is finished: 4A-03 → 4A-04 → CORE-01 → 4A-05 → 4A-10, all done. What 4A-10 unblocks is only half of 4A-11 — `griff swang verify` also needs 4A-09, which needs the parser, so the next real work is 4A-02 → INF-04 → INF-06 → 4A-06. Level 2 is **not** frozen and Phase 4A is **not** closed. Neither is implied by a CLI surface over a finished writer. The decision log gains the one decision this task actually made: `Score.loss` is rendered on both surfaces, and neither may consume the other. It reads as redundancy and is not — stdout carries a canonical fact the grammar owns, stderr a courtesy to whoever is watching the terminal. Suppressing the block because a human had already been told would make the canonical text depend on who was looking at it. Verified: 16 census witnesses still green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018vdRzztKXy8bA16tEzwgLE
Comments and docs only. No production behaviour changes; the only non-comment edit in Rust is the wording of one assertion message. Independent review found the closure overclaiming its own surface. Several places promised "one line per import warning" on stderr. That is not something this task can promise: `ImportWarning::Other(String)` is unrestricted and may contain an embedded LF — SWG-4A-05 pinned exactly that case in the exact text, where §6.5's frozen escape policy turns it into `\n` so a warning stays one physical line *there*. The terminal rendering has no such grammar, and `describe_warning` hands an `Other` message through verbatim, so one warning entry can legitimately occupy three lines of stderr. The fix is the sentence, not the code. Inventing an escaping or sanitization policy for human stderr in order to make a docstring true would be adding a contract nobody asked for, to a surface the task deliberately left loose — 4A-10's obligation is that warnings *go to stderr*, not that they arrive one per line. The invariant that is real, and is tested, is one rendered entry per `LossReport` element in vector order. No new test: the point is precisely not to freeze a stderr line policy. The decision log gains a continuation entry rather than an edit to the existing one — the overclaim was made, and a record that quietly stopped having been wrong would be a worse record. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018vdRzztKXy8bA16tEzwgLE
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
SWG-4A-10 —
griff swang dumpImport runs through the existing adapters; the canonical level-2 exact text
goes to stdout and only to stdout; import warnings and diagnostics go to
stderr. No hidden normalization, and two runs of one file produce
byte-identical output.
A CLI surface over a writer that was finished in SWG-4A-05 — not another
slice of it.
griff-swangand the importers are untouched: the diff is onenew
griff-climodule, one clap subcommand, one dispatch arm, oneCliErrorvariant, and this suite.Level 2 is not frozen and Phase 4A is not closed.
Commits (no squash — the order is the evidence)
f712131ec17096griff swang dump82fc5191c559464594ef1The RED is a real one. Commit 1 changes nothing under
cli/src/, so thesuite cannot compile:
and the binary agrees at runtime:
The one design decision
griff_cli::swang_dump::dump_scorereturns both surfaces together:write_scoreruns to completion before the value exists, so a refusalproduces no
DumpOutputat all. That makes no partial documentstructural rather than a discipline someone has to remember;
mainonlyroutes two finished surfaces to two streams. No second formatter appears
anywhere — the census already decided every byte.
Score.lossis rendered on both surfaces, deliberately. It is acanonical fact the exact text is obliged to carry (§2.8); the stderr entry
is a courtesy to whoever is watching the terminal. Suppressing the block
because a human had already been told would make the canonical text depend
on who was looking at it, and would undo what SWG-4A-05 spent twenty
mutations establishing.
The contract, one test each
.gpfixture written by theguitarprocrate's own serializermidly-built fixture, same entry pointwrite_score(import_score_auto(bytes))track_name_invalid_utf8present in stdout, described on stderr, document absent from stderrswang_dumpseam (see below)write_score(import_score_auto(bytes)), never a hand-copied goldenLossReportvector orderRow 8 is the one that makes this a transport test rather than a second
formatter agreeing with the first about something wrong.
Both fixtures come from encoders independent of the code under test.
guitarprois a new dev-dependency ofgriff-clionly — the crate isalready in the tree through
griff-core's defaultgpfeature, so this addstest-time access, not third-party code.
Row 7 needed a finding to state honestly
No file reaching
dumpis refused by the writer. The MIDI reader rejectsppqn 0 outright, and a zero meter numerator is silently normalised to 4/4
long before
write_scoresees it — both probed rather than assumed. So therefusal is proved at the library seam, and the unreachability is written
down instead of being papered over with a fabricated fixture.
Falsification: eleven mutations
Each probe was applied to
cli/src/swang_dump.rsorcli/src/main.rsinisolation and the whole
griff-clisuite re-run. The classifier is thefive-state one inherited from #190 — including NOT REBUILT, which
refuses to give a verdict on a run whose output does not contain
Compiling griff-cli, because this container's filesystem hands outtimestamps coarse enough for cargo to test the previous probe's binary.
Initial run: 10 CAUGHT / 1 SURVIVED.
The survivor was
D3 sort the warning entries. Nothing in the suite lookedat their order — the matrix only counted them — so a rendering that
alphabetised the loss report would have passed.
LossReportappends and theexact walker compares positionally (§2.7); the stderr entries are a
rendering of that vector, so they inherit its order rather than choosing
one. The guard is commit
82fc519, with a fixture built to see thedifference: eleven tracks, invalid UTF-8 names at raw indices 2 and 10, so
"track 10"sorts before"track 2"— and the test asserts sorted ≠vector order, which is what stops it passing vacuously if the fixture is
ever weakened.
Final rerun: 11 CAUGHT / 0 SURVIVED / 0 NOT REBUILT / 0 INVALID.
lossonce stderr has seen ita_dumped_score_carries_its_document_and_its_warnings_together,an_import_warning_is_reported_to_stderr_and_kept_in_the_document,the_cli_prints_exactly_what_the_writer_produceslossa_dumped_score_carries_its_document_and_its_warnings_together,an_import_warning_is_reported_to_stderr_and_kept_in_the_document,the_warning_lines_keep_the_loss_report_order82fc519)the_warning_lines_keep_the_loss_report_ordera_dumped_score_carries_its_document_and_its_warnings_together,stdout_carries_the_document_and_no_cli_chatter,the_cli_prints_exactly_what_the_writer_producesa_dumped_score_carries_its_document_and_its_warnings_together,a_guitar_pro_file_dumps_a_canonical_level_two_document,a_midi_file_dumps_a_canonical_level_two_document,stdout_carries_the_document_and_no_cli_chatter,the_cli_prints_exactly_what_the_writer_producesa_guitar_pro_file_dumps_a_canonical_level_two_document,a_midi_file_dumps_a_canonical_level_two_document,an_import_warning_is_reported_to_stderr_and_kept_in_the_document,stdout_carries_the_document_and_no_cli_chatter,the_cli_prints_exactly_what_the_writer_producesan_import_warning_is_reported_to_stderr_and_kept_in_the_document,the_cli_prints_exactly_what_the_writer_produces,the_warning_lines_keep_the_loss_report_ordera_guitar_pro_file_dumps_a_canonical_level_two_document,a_midi_file_dumps_a_canonical_level_two_document,an_unimportable_file_fails_with_a_diagnostic_and_no_document,stdout_carries_the_document_and_no_cli_chatter,the_cli_prints_exactly_what_the_writer_producesan_unimportable_file_fails_with_a_diagnostic_and_no_documentstdout_carries_the_document_and_no_cli_chatter,the_cli_prints_exactly_what_the_writer_producesstdout_carries_the_document_and_no_cli_chatter,the_cli_prints_exactly_what_the_writer_producesThe review round (commit 5)
Independent review found the closure overclaiming its own surface, not a
production defect. Several places promised "one line per import warning" on
stderr.
ImportWarning::Other(String)is unrestricted and may contain anembedded LF — SWG-4A-05 pinned exactly that case in the exact text, where
§6.5's frozen escape policy keeps a warning on one physical line there.
The terminal rendering has no such grammar and
describe_warningpasses anOthermessage through verbatim, so one entry can legitimately occupy threelines of stderr.
The fix is the sentence, not the code. Inventing an escaping or sanitization
policy for human stderr to make a docstring true would be adding a contract
nobody asked for: 4A-10's obligation is that warnings go to stderr, not
that they arrive one per line. The invariant that is real, and tested, is
one rendered entry per
LossReportelement in vector order. No new test —the point is precisely not to freeze a stderr line policy.
decisions.log.mdgets a continuation entry rather than an edit to the existing one.
Checks
cargo fmt --all --checkandcargo clippy --workspace --all-targetsclean.
cargo check --workspace --all-targetsclean, cockpit/preview/pluginincluded, via a stub
alsa.pc(libasound is absent here).fuzz/pinsnightly and is left to CI.
Out of scope, deliberately
The parser lane (4A-02, INF-04, INF-06, 4A-06+),
ExactScoreDocument,ScoreBuilder,griff swang verify(4A-11 — this satisfies its CLI-sidedependency only; it still needs 4A-09), CORE-02 sealing, stdin support,
machine-readable diagnostics, any new normalization, importer behaviour, or
exact-text syntax, and the level-2 freeze.
Also deliberately untouched: the task-index rows for 4A-03 and 4A-04 are
missing the
*(done)*marker their own sections and the dependency mapcarry. Pre-existing bookkeeping, unrelated to this task.
Generated by Claude Code