feat(config): T-1283 — the godot and visual domains
reach godot parse-sweep / cold-parse, reach visual diff / blank-check / thumbnail. Five scripts retired, and the callers rewired — tests/run-visual invoked three of them by path at four sites, which is a wider blast radius than the make targets were. The godot pair were grep pipelines encoding five hard-won lessons as comments nobody could test. They are Python filters now, with the reasons attached, and the engine invocation is a guarded exec. Verified on the real client: 229 scripts, clean. Their three not-ok states stay distinct, because only one is a verdict about the code. An engine that crashed or is missing is not a parse failure — reporting it as one blames the tree for a broken toolchain. A sweep that emitted no completion marker checked nothing, and zero errors from a check that never ran reads as clean, which is the false-green the sweep exists to close. The deliberate asymmetry between the two checks is preserved and documented: cold-parse filters "Cannot infer the type", the sweep does not, because that suppression is why cold-parse stayed silent about a helper that genuinely does not parse. All three visual scripts carried the same root bug as validate-checklist: Path(__file__).parent.parent, correct at tooling/ and two levels too deep at tooling/domains/visual. Fixed during the move rather than after, having learned that it fails silently — paths resolve to nothing, the work appears to have nothing to do, and the tool reports success. Three domains now where that would have shipped a false pass. Two bugs my own transformation introduced, both found by running rather than reading. Multi-line print(..., file=sys.stderr) became console.event(..., file=sys.stderr), and console puts unknown kwargs into the payload — a file object would have reached json.dumps at the exact moment something was already being reported as an error. And the replacement script wrote escaped quotes into three files. Mechanical transformations need mechanical verification. sys.exit removed from four sites: a service must not end the process. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Claude Opus 5
@@ -51,7 +51,6 @@
|
||||
"Bash(godot4:*)",
|
||||
"Bash(gdformat:*)",
|
||||
"Bash(tooling/atlas:*)",
|
||||
"Bash(tooling/godot-cold-parse:*)",
|
||||
"Bash(tooling/pr-watchlist-diff:*)",
|
||||
"Bash(chmod *)",
|
||||
"Bash(ls *)",
|
||||
|
||||
@@ -109,9 +109,9 @@ fi
|
||||
# including the ~half of the codebase no test ever loads. It is also the only
|
||||
# gate that covers them at all: godot-cold-parse is skill-only (never invoked
|
||||
# by this hook) and sees only the startup path regardless.
|
||||
if [ "$CLIENT_CHANGED" -gt 0 ] && [ -x "$REPO_ROOT/tooling/godot-parse-sweep" ]; then
|
||||
echo "pre-push: sweeping GDScript parse (tooling/godot-parse-sweep)..."
|
||||
if ! "$REPO_ROOT/tooling/godot-parse-sweep"; then
|
||||
if [ "$CLIENT_CHANGED" -gt 0 ] && command -v reach >/dev/null 2>&1; then
|
||||
echo "pre-push: sweeping GDScript parse (reach godot parse-sweep)..."
|
||||
if ! reach --no-input godot parse-sweep; then
|
||||
echo "pre-push: parse sweep FAILED — a script does not parse"
|
||||
fail_check "GDScript parse sweep"
|
||||
else
|
||||
|
||||
@@ -1,2 +1,41 @@
|
||||
INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06G5GWDPQ95RSCKK51K8DTYRCM', 'status', 'backlog', 'in_progress', NULL, '2026-09-01 15:22:13', '2026-09-01 15:22:13.360', '2026-09-01 15:22:13.360', NULL, '524f353821f4dc123ec999ca0e84f25e', 2) ON CONFLICT(hash) DO NOTHING;
|
||||
INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06G5GWDPQ95RSCKK51K8DTYRCM', 'status', 'in_progress', 'in_progress', NULL, '2026-09-01 15:22:27', '2026-09-01 15:22:27.856', '2026-09-01 15:22:27.856', NULL, '3c5c1770ada99b9f3c0822537836737e', 2) ON CONFLICT(hash) DO NOTHING;
|
||||
INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06G5GWDPQ95RSCKK51K8DTYRCM', 'description', 'validate-content, validate-checklist and validate-ron into domains/validate/. Standard port acceptance as defined on T-1281: transport-agnostic service, logic-free router, every command decorated, a parity test per verb against the old script, old scripts retired once parity passes, and that domain''s tooling make targets retired rather than wrapped. Note validate-content is wired into the PRE-COMMIT hook (via make validate-content) as well as being a make target, so the hook is part of this port''s blast radius, not just the Makefile — and a pre-commit failure is felt on every commit rather than every push, so getting the exit codes right matters more here than for a push-only gate. check-fact-ids is ALSO in the pre-commit hook but belongs to the check domain (T-1281); coordinate so the hook is edited once rather than twice.', 'validate-content, validate-checklist and validate-ron into domains/validate/. Standard port acceptance as defined on T-1281: transport-agnostic service, logic-free router, every command decorated, a parity test per verb against the old script, old scripts retired once parity passes, and that domain''s tooling make targets retired rather than wrapped. Note validate-content is wired into the PRE-COMMIT hook (via make validate-content) as well as being a make target, so the hook is part of this port''s blast radius, not just the Makefile — and a pre-commit failure is felt on every commit rather than every push, so getting the exit codes right matters more here than for a push-only gate. check-fact-ids is ALSO in the pre-commit hook but belongs to the check domain (T-1281); coordinate so the hook is edited once rather than twice.
|
||||
|
||||
DONE 2026-09-02. reach validate content / checklist / ron / name-collisions. All three old scripts retired, make targets retired, permission entry dropped.
|
||||
|
||||
PRINTS GO THROUGH THE LOGGING SINK, per Jeroen mid-ticket: "print statements go through the logging decorator" and "it already is a collector, the logging sink". My first pass added a per-module _lines list; that was redundant, because console IS the collector. The 48 sites in content.py and 24 in checklist.py now emit console events, message strings and order unchanged. Consequence beyond tidiness: a long content validation STREAMS as it runs rather than going quiet and dumping at the end, and every line carries the invocation''s job id.
|
||||
|
||||
validate-ron WAS THREE LANGUAGES DEEP — bash dispatching on a flag, a Python heredoc doing collision detection, cargo run for schema validation. Logic embedded in a shell string cannot be imported, tested, or found by anything that indexes Python; that is the clearest case yet for the rewrite decision. The heredoc became Python, the cargo call became a guarded exec through core/process.run with missing_fix naming `make setup-rust`.
|
||||
It also SPLIT INTO TWO VERBS. --check-name-collisions answered a different question from the default path — whether the SET of cultures is coherent versus whether ONE file is well-formed — and the old script had to branch on the flag before doing anything. Two verbs, no branch.
|
||||
|
||||
THE MOVE BROKE SOMETHING, QUIETLY, which is the argument for doing these one domain at a time. validate-checklist computed ROOT as Path(__file__).resolve().parent.parent — the repo root while the file lived at tooling/validate-checklist, and tooling/domains once moved. Its schema and gauntlet paths silently repointed at nothing, the gauntlet directory "did not exist", find_checklists returned [], and it reported "skipped (no content yet)" with exit 0. Caught only by running it beside the original: old exit 1 (schema not found), new exit 0. Fixed to config.repo_root(). Also converted load_schema''s sys.exit(1) to a ReachError — a service must not end the process, and the caller now gets a remedy instead of a bare 1.
|
||||
|
||||
PARITY on the live tree: content reproduces the original byte for byte including its counts (0 validated, 7 skipped, 13 errors); name-collisions likewise (OK across 3 cultures). tooling/test_validate.py pins what those runs cannot reach — collision DETECTION (the repo currently has none), the comment-stripping rule, the empty and missing directory cases, and the two argument errors. Proven to fail by removing comment stripping, which tripped both the decoy assertion and the no-collision case.
|
||||
|
||||
TWO FINDINGS LEFT ALONE, deliberately:
|
||||
- validate-content FAILS on the live tree: 13 MISSING SCHEMA errors under server/content/_schema/. Pre-existing, unrelated to this port, and it means `make pre-pr-validate` has been failing. Not fixed here because writing 13 JSON schemas is content work, not a port. Worth its own ticket.
|
||||
- THIS TICKET''S DESCRIPTION IS WRONG about validate-content being in the pre-commit hook. That hook runs only check-fact-ids (ported in T-1281) and pql decisions validate. There was no shared hook edit to coordinate.', NULL, '2026-09-02 10:50:19', '2026-09-02 10:50:19.808', '2026-09-02 10:50:19.808', NULL, '73643365e7f51247d7b78442a67c4dfd', 2) ON CONFLICT(hash) DO NOTHING;
|
||||
INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06G5GWDPQ95RSCKK51K8DTYRCM', 'status', 'in_progress', 'done', NULL, '2026-09-02 10:50:19', '2026-09-02 10:50:19.825', '2026-09-02 10:50:19.825', NULL, '392efb133e220fafef2fe5edf9c921b7', 2) ON CONFLICT(hash) DO NOTHING;
|
||||
INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06G5GWEC9P1HNABW6S0BSNTZ2W', 'status', 'backlog', 'in_progress', NULL, '2026-09-02 10:53:39', '2026-09-02 10:53:39.241', '2026-09-02 10:53:39.241', NULL, 'b8fbe55d25644233f56e66e03b7e63eb', 2) ON CONFLICT(hash) DO NOTHING;
|
||||
INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06G5GWEC9P1HNABW6S0BSNTZ2W', 'description', 'Two small domains, filed together because each is two or three files and neither has surprises. godot: godot-parse-sweep and godot-cold-parse into domains/godot/. visual: visual-diff, visual-thumbnail and visual-blank-check into domains/visual/. Standard port acceptance as defined on T-1281. Worth knowing before starting: both domains shell out to external binaries — godot4 and image tooling respectively — so their services need a launcher, and core/process.py already has the spawn primitive from T-1277. Do NOT let each service grow its own subprocess handling; if what is needed is broader than what core/process.py offers, extend it there rather than twice locally. That is the difference between a shared substrate and two copies that drift. Also: these are the first ports whose commands could genuinely be slow (a parse sweep over the whole client), so they are the first real candidates for progress events via console.event — the streaming machinery exists and this is where it starts earning.', 'Two small domains, filed together because each is two or three files and neither has surprises. godot: godot-parse-sweep and godot-cold-parse into domains/godot/. visual: visual-diff, visual-thumbnail and visual-blank-check into domains/visual/. Standard port acceptance as defined on T-1281. Worth knowing before starting: both domains shell out to external binaries — godot4 and image tooling respectively — so their services need a launcher, and core/process.py already has the spawn primitive from T-1277. Do NOT let each service grow its own subprocess handling; if what is needed is broader than what core/process.py offers, extend it there rather than twice locally. That is the difference between a shared substrate and two copies that drift. Also: these are the first ports whose commands could genuinely be slow (a parse sweep over the whole client), so they are the first real candidates for progress events via console.event — the streaming machinery exists and this is where it starts earning.
|
||||
|
||||
DONE 2026-09-02. reach godot parse-sweep / cold-parse and reach visual diff / blank-check / thumbnail. Five old scripts retired, callers rewired.
|
||||
|
||||
GODOT — two bash scripts whose logic was grep pipelines encoding five separate hard-won lessons, each a comment nobody could test. Now Python filters with the reasons attached, and the engine invocation as a guarded exec. Verified on the real client: "godot-parse-sweep: clean — opened 229 scripts (0 unreadable dirs)".
|
||||
Three not-ok states kept DISTINCT, because only one of them is a verdict about the code: engine_failed (godot crashed or is missing — reporting that as a parse failure blames the tree for a broken toolchain), did_not_run (the sweep emitted no completion marker, so it checked nothing — zero errors from a check that never ran reads as clean, the false-green this tool exists to close), and genuine parse errors.
|
||||
The asymmetry between the two checks is preserved and documented: cold-parse filters "Cannot infer the type", the sweep does NOT. That suppression is why cold-parse stayed silent about a test helper that genuinely does not parse.
|
||||
|
||||
VISUAL — three Python scripts with argparse mains. argparse removed: a parser inside a service is a second transport layer, and the router declares the options now.
|
||||
|
||||
THE SAME ROOT BUG AS T-1282, in all three files. Every one computed Path(__file__).resolve().parent.parent — the repo root at tooling/, and tooling/domains/visual once moved, two levels too deep. Fixed to config.repo_root() during the move rather than after, having learned from validate-checklist that this fails SILENTLY: paths resolve to nothing, the work appears to have nothing to do, and the tool reports success. That is now three domains where a __file__-relative root would have shipped a false pass.
|
||||
|
||||
TWO BUGS MY OWN TRANSFORMATION INTRODUCED, both caught by running rather than reading:
|
||||
- Multi-line print(..., file=sys.stderr) became console.event(..., file=sys.stderr). console.event puts unknown kwargs into the event payload, so a file OBJECT would have gone to json.dumps — a crash at the moment something was already being reported as an error. Removed, level="error" instead.
|
||||
- The escaping in my replacement script wrote level=\"error\" into three files, which ruff caught as a syntax error.
|
||||
Neither was visible by inspection of the diff; both surfaced on the first real run. Mechanical transformations need mechanical verification.
|
||||
|
||||
sys.exit REMOVED from diff.py and thumbnail.py — four sites. A service must not end the process; they raise ReachError with a remedy now.
|
||||
|
||||
CALLERS REWIRED, and this is the wider blast radius than the make targets: tests/run-visual invoked visual-blank-check, visual-thumbnail and visual-diff by path at four sites, and the pre-push hook invoked godot-parse-sweep. All now call reach --no-input. .claude/settings.json loses the godot-cold-parse entry; Bash(reach *) covers it.', NULL, '2026-09-02 11:59:14', '2026-09-02 11:59:14.517', '2026-09-02 11:59:14.517', NULL, '9b3c9ad13e13e0ed35f1b856e53341b6', 2) ON CONFLICT(hash) DO NOTHING;
|
||||
INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06G5GWEC9P1HNABW6S0BSNTZ2W', 'status', 'in_progress', 'done', NULL, '2026-09-02 11:59:14', '2026-09-02 11:59:14.536', '2026-09-02 11:59:14.536', NULL, '74c5a448237be06cfc9b1b0697f059f2', 2) ON CONFLICT(hash) DO NOTHING;
|
||||
|
||||
@@ -0,0 +1 @@
|
||||
INSERT INTO ticket_idmap (record_id, ticket_id, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06G63FEA51J4JKE3A7473D3QZ0', 'T-1284', '2026-09-02 10:50:23.145', '2026-09-02 10:50:23.145', NULL, '74113524e3a280bd06875305a547349a', 2) ON CONFLICT(record_id) DO UPDATE SET ticket_id=excluded.ticket_id, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at >= ticket_idmap.updated_at;
|
||||
@@ -1,2 +1,76 @@
|
||||
INSERT INTO tickets (record_id, type, parent_record_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06G5GWDPQ95RSCKK51K8DTYRCM', 'task', '06G1S3D0M1TQW0GMFBBPQZG3ZM', 'Port the validate domain — content, checklist, ron', 'validate-content, validate-checklist and validate-ron into domains/validate/. Standard port acceptance as defined on T-1281: transport-agnostic service, logic-free router, every command decorated, a parity test per verb against the old script, old scripts retired once parity passes, and that domain''s tooling make targets retired rather than wrapped. Note validate-content is wired into the PRE-COMMIT hook (via make validate-content) as well as being a make target, so the hook is part of this port''s blast radius, not just the Makefile — and a pre-commit failure is felt on every commit rather than every push, so getting the exit codes right matters more here than for a push-only gate. check-fact-ids is ALSO in the pre-commit hook but belongs to the check domain (T-1281); coordinate so the hook is edited once rather than twice.', 'in_progress', 'medium', NULL, NULL, 'D-263', '2026-08-31 15:30:42.490', '2026-09-01 15:22:13.360', NULL, 'd8cc608ee1be87634289397ebae7fd61', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at >= tickets.updated_at;
|
||||
INSERT INTO tickets (record_id, type, parent_record_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06G5GWDPQ95RSCKK51K8DTYRCM', 'task', '06G1S3D0M1TQW0GMFBBPQZG3ZM', 'Port the validate domain — content, checklist, ron', 'validate-content, validate-checklist and validate-ron into domains/validate/. Standard port acceptance as defined on T-1281: transport-agnostic service, logic-free router, every command decorated, a parity test per verb against the old script, old scripts retired once parity passes, and that domain''s tooling make targets retired rather than wrapped. Note validate-content is wired into the PRE-COMMIT hook (via make validate-content) as well as being a make target, so the hook is part of this port''s blast radius, not just the Makefile — and a pre-commit failure is felt on every commit rather than every push, so getting the exit codes right matters more here than for a push-only gate. check-fact-ids is ALSO in the pre-commit hook but belongs to the check domain (T-1281); coordinate so the hook is edited once rather than twice.', 'in_progress', 'medium', NULL, NULL, 'D-263', '2026-08-31 15:30:42.490', '2026-09-01 15:22:27.856', NULL, '89c48e98147081cb5465e7373234ec9f', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at >= tickets.updated_at;
|
||||
INSERT INTO tickets (record_id, type, parent_record_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06G5GWDPQ95RSCKK51K8DTYRCM', 'task', '06G1S3D0M1TQW0GMFBBPQZG3ZM', 'Port the validate domain — content, checklist, ron', 'validate-content, validate-checklist and validate-ron into domains/validate/. Standard port acceptance as defined on T-1281: transport-agnostic service, logic-free router, every command decorated, a parity test per verb against the old script, old scripts retired once parity passes, and that domain''s tooling make targets retired rather than wrapped. Note validate-content is wired into the PRE-COMMIT hook (via make validate-content) as well as being a make target, so the hook is part of this port''s blast radius, not just the Makefile — and a pre-commit failure is felt on every commit rather than every push, so getting the exit codes right matters more here than for a push-only gate. check-fact-ids is ALSO in the pre-commit hook but belongs to the check domain (T-1281); coordinate so the hook is edited once rather than twice.
|
||||
|
||||
DONE 2026-09-02. reach validate content / checklist / ron / name-collisions. All three old scripts retired, make targets retired, permission entry dropped.
|
||||
|
||||
PRINTS GO THROUGH THE LOGGING SINK, per Jeroen mid-ticket: "print statements go through the logging decorator" and "it already is a collector, the logging sink". My first pass added a per-module _lines list; that was redundant, because console IS the collector. The 48 sites in content.py and 24 in checklist.py now emit console events, message strings and order unchanged. Consequence beyond tidiness: a long content validation STREAMS as it runs rather than going quiet and dumping at the end, and every line carries the invocation''s job id.
|
||||
|
||||
validate-ron WAS THREE LANGUAGES DEEP — bash dispatching on a flag, a Python heredoc doing collision detection, cargo run for schema validation. Logic embedded in a shell string cannot be imported, tested, or found by anything that indexes Python; that is the clearest case yet for the rewrite decision. The heredoc became Python, the cargo call became a guarded exec through core/process.run with missing_fix naming `make setup-rust`.
|
||||
It also SPLIT INTO TWO VERBS. --check-name-collisions answered a different question from the default path — whether the SET of cultures is coherent versus whether ONE file is well-formed — and the old script had to branch on the flag before doing anything. Two verbs, no branch.
|
||||
|
||||
THE MOVE BROKE SOMETHING, QUIETLY, which is the argument for doing these one domain at a time. validate-checklist computed ROOT as Path(__file__).resolve().parent.parent — the repo root while the file lived at tooling/validate-checklist, and tooling/domains once moved. Its schema and gauntlet paths silently repointed at nothing, the gauntlet directory "did not exist", find_checklists returned [], and it reported "skipped (no content yet)" with exit 0. Caught only by running it beside the original: old exit 1 (schema not found), new exit 0. Fixed to config.repo_root(). Also converted load_schema''s sys.exit(1) to a ReachError — a service must not end the process, and the caller now gets a remedy instead of a bare 1.
|
||||
|
||||
PARITY on the live tree: content reproduces the original byte for byte including its counts (0 validated, 7 skipped, 13 errors); name-collisions likewise (OK across 3 cultures). tooling/test_validate.py pins what those runs cannot reach — collision DETECTION (the repo currently has none), the comment-stripping rule, the empty and missing directory cases, and the two argument errors. Proven to fail by removing comment stripping, which tripped both the decoy assertion and the no-collision case.
|
||||
|
||||
TWO FINDINGS LEFT ALONE, deliberately:
|
||||
- validate-content FAILS on the live tree: 13 MISSING SCHEMA errors under server/content/_schema/. Pre-existing, unrelated to this port, and it means `make pre-pr-validate` has been failing. Not fixed here because writing 13 JSON schemas is content work, not a port. Worth its own ticket.
|
||||
- THIS TICKET''S DESCRIPTION IS WRONG about validate-content being in the pre-commit hook. That hook runs only check-fact-ids (ported in T-1281) and pql decisions validate. There was no shared hook edit to coordinate.', 'in_progress', 'medium', NULL, NULL, 'D-263', '2026-08-31 15:30:42.490', '2026-09-02 10:50:19.803', NULL, '137939508f1a89fb45ebfa2fe7580a8e', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at >= tickets.updated_at;
|
||||
INSERT INTO tickets (record_id, type, parent_record_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06G5GWDPQ95RSCKK51K8DTYRCM', 'task', '06G1S3D0M1TQW0GMFBBPQZG3ZM', 'Port the validate domain — content, checklist, ron', 'validate-content, validate-checklist and validate-ron into domains/validate/. Standard port acceptance as defined on T-1281: transport-agnostic service, logic-free router, every command decorated, a parity test per verb against the old script, old scripts retired once parity passes, and that domain''s tooling make targets retired rather than wrapped. Note validate-content is wired into the PRE-COMMIT hook (via make validate-content) as well as being a make target, so the hook is part of this port''s blast radius, not just the Makefile — and a pre-commit failure is felt on every commit rather than every push, so getting the exit codes right matters more here than for a push-only gate. check-fact-ids is ALSO in the pre-commit hook but belongs to the check domain (T-1281); coordinate so the hook is edited once rather than twice.
|
||||
|
||||
DONE 2026-09-02. reach validate content / checklist / ron / name-collisions. All three old scripts retired, make targets retired, permission entry dropped.
|
||||
|
||||
PRINTS GO THROUGH THE LOGGING SINK, per Jeroen mid-ticket: "print statements go through the logging decorator" and "it already is a collector, the logging sink". My first pass added a per-module _lines list; that was redundant, because console IS the collector. The 48 sites in content.py and 24 in checklist.py now emit console events, message strings and order unchanged. Consequence beyond tidiness: a long content validation STREAMS as it runs rather than going quiet and dumping at the end, and every line carries the invocation''s job id.
|
||||
|
||||
validate-ron WAS THREE LANGUAGES DEEP — bash dispatching on a flag, a Python heredoc doing collision detection, cargo run for schema validation. Logic embedded in a shell string cannot be imported, tested, or found by anything that indexes Python; that is the clearest case yet for the rewrite decision. The heredoc became Python, the cargo call became a guarded exec through core/process.run with missing_fix naming `make setup-rust`.
|
||||
It also SPLIT INTO TWO VERBS. --check-name-collisions answered a different question from the default path — whether the SET of cultures is coherent versus whether ONE file is well-formed — and the old script had to branch on the flag before doing anything. Two verbs, no branch.
|
||||
|
||||
THE MOVE BROKE SOMETHING, QUIETLY, which is the argument for doing these one domain at a time. validate-checklist computed ROOT as Path(__file__).resolve().parent.parent — the repo root while the file lived at tooling/validate-checklist, and tooling/domains once moved. Its schema and gauntlet paths silently repointed at nothing, the gauntlet directory "did not exist", find_checklists returned [], and it reported "skipped (no content yet)" with exit 0. Caught only by running it beside the original: old exit 1 (schema not found), new exit 0. Fixed to config.repo_root(). Also converted load_schema''s sys.exit(1) to a ReachError — a service must not end the process, and the caller now gets a remedy instead of a bare 1.
|
||||
|
||||
PARITY on the live tree: content reproduces the original byte for byte including its counts (0 validated, 7 skipped, 13 errors); name-collisions likewise (OK across 3 cultures). tooling/test_validate.py pins what those runs cannot reach — collision DETECTION (the repo currently has none), the comment-stripping rule, the empty and missing directory cases, and the two argument errors. Proven to fail by removing comment stripping, which tripped both the decoy assertion and the no-collision case.
|
||||
|
||||
TWO FINDINGS LEFT ALONE, deliberately:
|
||||
- validate-content FAILS on the live tree: 13 MISSING SCHEMA errors under server/content/_schema/. Pre-existing, unrelated to this port, and it means `make pre-pr-validate` has been failing. Not fixed here because writing 13 JSON schemas is content work, not a port. Worth its own ticket.
|
||||
- THIS TICKET''S DESCRIPTION IS WRONG about validate-content being in the pre-commit hook. That hook runs only check-fact-ids (ported in T-1281) and pql decisions validate. There was no shared hook edit to coordinate.', 'done', 'medium', NULL, NULL, 'D-263', '2026-08-31 15:30:42.490', '2026-09-02 10:50:19.824', NULL, 'e6eed1166d37970c10376ba6ad9b2270', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at >= tickets.updated_at;
|
||||
INSERT INTO tickets (record_id, type, parent_record_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06G63FEA51J4JKE3A7473D3QZ0', 'task', '06FB0TNSRZXCHGS16BFHSSGSV4', 'Write the 13 missing content JSON schemas', 'reach validate content currently FAILS on the live tree with 13 MISSING SCHEMA errors: the campaigns tree references schemas that do not exist under server/content/_schema/ — system.schema.json among them. Found 2026-09-02 while porting the validator (T-1282); PRE-EXISTING and unrelated to that port, which reproduces the failure byte for byte because it reproduces the original''s behaviour. Consequence: make pre-pr-validate has been failing, so that chain has not been a working gate. Writing the schemas is content work, not tooling work, which is why it was not folded into the port. Run reach validate content for the current list.', 'backlog', 'medium', NULL, NULL, NULL, '2026-09-02 10:50:23.144', '2026-09-02 10:50:23.144', NULL, '64e70334d534198b9b07160eced5d909', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at >= tickets.updated_at;
|
||||
INSERT INTO tickets (record_id, type, parent_record_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06G5GWEC9P1HNABW6S0BSNTZ2W', 'task', '06G1S3D0M1TQW0GMFBBPQZG3ZM', 'Port the godot and visual domains', 'Two small domains, filed together because each is two or three files and neither has surprises. godot: godot-parse-sweep and godot-cold-parse into domains/godot/. visual: visual-diff, visual-thumbnail and visual-blank-check into domains/visual/. Standard port acceptance as defined on T-1281. Worth knowing before starting: both domains shell out to external binaries — godot4 and image tooling respectively — so their services need a launcher, and core/process.py already has the spawn primitive from T-1277. Do NOT let each service grow its own subprocess handling; if what is needed is broader than what core/process.py offers, extend it there rather than twice locally. That is the difference between a shared substrate and two copies that drift. Also: these are the first ports whose commands could genuinely be slow (a parse sweep over the whole client), so they are the first real candidates for progress events via console.event — the streaming machinery exists and this is where it starts earning.', 'in_progress', 'medium', NULL, NULL, 'D-263', '2026-08-31 15:30:48.013', '2026-09-02 10:53:39.240', NULL, '9a437c8d09c93b7edbf080000399952a', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at >= tickets.updated_at;
|
||||
INSERT INTO tickets (record_id, type, parent_record_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06G5GWEC9P1HNABW6S0BSNTZ2W', 'task', '06G1S3D0M1TQW0GMFBBPQZG3ZM', 'Port the godot and visual domains', 'Two small domains, filed together because each is two or three files and neither has surprises. godot: godot-parse-sweep and godot-cold-parse into domains/godot/. visual: visual-diff, visual-thumbnail and visual-blank-check into domains/visual/. Standard port acceptance as defined on T-1281. Worth knowing before starting: both domains shell out to external binaries — godot4 and image tooling respectively — so their services need a launcher, and core/process.py already has the spawn primitive from T-1277. Do NOT let each service grow its own subprocess handling; if what is needed is broader than what core/process.py offers, extend it there rather than twice locally. That is the difference between a shared substrate and two copies that drift. Also: these are the first ports whose commands could genuinely be slow (a parse sweep over the whole client), so they are the first real candidates for progress events via console.event — the streaming machinery exists and this is where it starts earning.
|
||||
|
||||
DONE 2026-09-02. reach godot parse-sweep / cold-parse and reach visual diff / blank-check / thumbnail. Five old scripts retired, callers rewired.
|
||||
|
||||
GODOT — two bash scripts whose logic was grep pipelines encoding five separate hard-won lessons, each a comment nobody could test. Now Python filters with the reasons attached, and the engine invocation as a guarded exec. Verified on the real client: "godot-parse-sweep: clean — opened 229 scripts (0 unreadable dirs)".
|
||||
Three not-ok states kept DISTINCT, because only one of them is a verdict about the code: engine_failed (godot crashed or is missing — reporting that as a parse failure blames the tree for a broken toolchain), did_not_run (the sweep emitted no completion marker, so it checked nothing — zero errors from a check that never ran reads as clean, the false-green this tool exists to close), and genuine parse errors.
|
||||
The asymmetry between the two checks is preserved and documented: cold-parse filters "Cannot infer the type", the sweep does NOT. That suppression is why cold-parse stayed silent about a test helper that genuinely does not parse.
|
||||
|
||||
VISUAL — three Python scripts with argparse mains. argparse removed: a parser inside a service is a second transport layer, and the router declares the options now.
|
||||
|
||||
THE SAME ROOT BUG AS T-1282, in all three files. Every one computed Path(__file__).resolve().parent.parent — the repo root at tooling/, and tooling/domains/visual once moved, two levels too deep. Fixed to config.repo_root() during the move rather than after, having learned from validate-checklist that this fails SILENTLY: paths resolve to nothing, the work appears to have nothing to do, and the tool reports success. That is now three domains where a __file__-relative root would have shipped a false pass.
|
||||
|
||||
TWO BUGS MY OWN TRANSFORMATION INTRODUCED, both caught by running rather than reading:
|
||||
- Multi-line print(..., file=sys.stderr) became console.event(..., file=sys.stderr). console.event puts unknown kwargs into the event payload, so a file OBJECT would have gone to json.dumps — a crash at the moment something was already being reported as an error. Removed, level="error" instead.
|
||||
- The escaping in my replacement script wrote level=\"error\" into three files, which ruff caught as a syntax error.
|
||||
Neither was visible by inspection of the diff; both surfaced on the first real run. Mechanical transformations need mechanical verification.
|
||||
|
||||
sys.exit REMOVED from diff.py and thumbnail.py — four sites. A service must not end the process; they raise ReachError with a remedy now.
|
||||
|
||||
CALLERS REWIRED, and this is the wider blast radius than the make targets: tests/run-visual invoked visual-blank-check, visual-thumbnail and visual-diff by path at four sites, and the pre-push hook invoked godot-parse-sweep. All now call reach --no-input. .claude/settings.json loses the godot-cold-parse entry; Bash(reach *) covers it.', 'in_progress', 'medium', NULL, NULL, 'D-263', '2026-08-31 15:30:48.013', '2026-09-02 11:59:14.517', NULL, '9789ab592ea31d9474bca2166d1fb79e', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at >= tickets.updated_at;
|
||||
INSERT INTO tickets (record_id, type, parent_record_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06G5GWEC9P1HNABW6S0BSNTZ2W', 'task', '06G1S3D0M1TQW0GMFBBPQZG3ZM', 'Port the godot and visual domains', 'Two small domains, filed together because each is two or three files and neither has surprises. godot: godot-parse-sweep and godot-cold-parse into domains/godot/. visual: visual-diff, visual-thumbnail and visual-blank-check into domains/visual/. Standard port acceptance as defined on T-1281. Worth knowing before starting: both domains shell out to external binaries — godot4 and image tooling respectively — so their services need a launcher, and core/process.py already has the spawn primitive from T-1277. Do NOT let each service grow its own subprocess handling; if what is needed is broader than what core/process.py offers, extend it there rather than twice locally. That is the difference between a shared substrate and two copies that drift. Also: these are the first ports whose commands could genuinely be slow (a parse sweep over the whole client), so they are the first real candidates for progress events via console.event — the streaming machinery exists and this is where it starts earning.
|
||||
|
||||
DONE 2026-09-02. reach godot parse-sweep / cold-parse and reach visual diff / blank-check / thumbnail. Five old scripts retired, callers rewired.
|
||||
|
||||
GODOT — two bash scripts whose logic was grep pipelines encoding five separate hard-won lessons, each a comment nobody could test. Now Python filters with the reasons attached, and the engine invocation as a guarded exec. Verified on the real client: "godot-parse-sweep: clean — opened 229 scripts (0 unreadable dirs)".
|
||||
Three not-ok states kept DISTINCT, because only one of them is a verdict about the code: engine_failed (godot crashed or is missing — reporting that as a parse failure blames the tree for a broken toolchain), did_not_run (the sweep emitted no completion marker, so it checked nothing — zero errors from a check that never ran reads as clean, the false-green this tool exists to close), and genuine parse errors.
|
||||
The asymmetry between the two checks is preserved and documented: cold-parse filters "Cannot infer the type", the sweep does NOT. That suppression is why cold-parse stayed silent about a test helper that genuinely does not parse.
|
||||
|
||||
VISUAL — three Python scripts with argparse mains. argparse removed: a parser inside a service is a second transport layer, and the router declares the options now.
|
||||
|
||||
THE SAME ROOT BUG AS T-1282, in all three files. Every one computed Path(__file__).resolve().parent.parent — the repo root at tooling/, and tooling/domains/visual once moved, two levels too deep. Fixed to config.repo_root() during the move rather than after, having learned from validate-checklist that this fails SILENTLY: paths resolve to nothing, the work appears to have nothing to do, and the tool reports success. That is now three domains where a __file__-relative root would have shipped a false pass.
|
||||
|
||||
TWO BUGS MY OWN TRANSFORMATION INTRODUCED, both caught by running rather than reading:
|
||||
- Multi-line print(..., file=sys.stderr) became console.event(..., file=sys.stderr). console.event puts unknown kwargs into the event payload, so a file OBJECT would have gone to json.dumps — a crash at the moment something was already being reported as an error. Removed, level="error" instead.
|
||||
- The escaping in my replacement script wrote level=\"error\" into three files, which ruff caught as a syntax error.
|
||||
Neither was visible by inspection of the diff; both surfaced on the first real run. Mechanical transformations need mechanical verification.
|
||||
|
||||
sys.exit REMOVED from diff.py and thumbnail.py — four sites. A service must not end the process; they raise ReachError with a remedy now.
|
||||
|
||||
CALLERS REWIRED, and this is the wider blast radius than the make targets: tests/run-visual invoked visual-blank-check, visual-thumbnail and visual-diff by path at four sites, and the pre-push hook invoked godot-parse-sweep. All now call reach --no-input. .claude/settings.json loses the godot-cold-parse entry; Bash(reach *) covers it.', 'done', 'medium', NULL, NULL, 'D-263', '2026-08-31 15:30:48.013', '2026-09-02 11:59:14.536', NULL, 'a3a0b27a427faca6f2c47ae6a9a6f9a5', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at >= tickets.updated_at;
|
||||
|
||||
+5
-5
@@ -244,7 +244,7 @@ if [[ "$MODE" == "screenshot" ]]; then
|
||||
# genuinely renders nothing is a legitimate thing to want to look at
|
||||
# (that is how the empty deep rungs were found). But say so out loud,
|
||||
# because file size alone reads as success.
|
||||
"$ROOT/tooling/visual-blank-check" "$PNG" --quiet || true
|
||||
reach --no-input visual blank-check "$PNG" --quiet || true
|
||||
else
|
||||
echo "Error: capture failed — $PNG not found" >&2
|
||||
exit 1
|
||||
@@ -267,8 +267,8 @@ if [[ "$MODE" == "movie" ]]; then
|
||||
echo "Flow: $FRAME_COUNT frames in $FLOW_DIR"
|
||||
|
||||
# Generate contact sheet if visual-thumbnail is available
|
||||
if [[ -x "$ROOT/tooling/visual-thumbnail" ]]; then
|
||||
"$ROOT/tooling/visual-thumbnail" "$FLOW_DIR" --config "$CONFIG"
|
||||
if command -v reach >/dev/null 2>&1; then
|
||||
reach --no-input visual thumbnail "$FLOW_DIR" --config "$CONFIG"
|
||||
SHEET="$FLOW_DIR/${TARGET}_sheet.png"
|
||||
[[ -f "$SHEET" ]] && echo "Contact sheet: $SHEET"
|
||||
fi
|
||||
@@ -345,7 +345,7 @@ for scenario in "${SCENARIOS[@]}"; do
|
||||
# Checked in BOTH modes, and the update mode matters most: refusing to
|
||||
# RECORD a blank golden is what stops the trap being re-armed.
|
||||
set +e
|
||||
BLANK_OUT=$("$ROOT/tooling/visual-blank-check" "$CAPTURED" 2>&1)
|
||||
BLANK_OUT=$(reach --no-input visual blank-check "$CAPTURED" 2>&1)
|
||||
BLANK_RC=$?
|
||||
set -e
|
||||
if [[ $BLANK_RC -ne 0 ]]; then
|
||||
@@ -368,7 +368,7 @@ for scenario in "${SCENARIOS[@]}"; do
|
||||
|
||||
# Compare
|
||||
set +e
|
||||
DIFF_OUT=$("$ROOT/tooling/visual-diff" "$GOLDEN" "$CAPTURED" \
|
||||
DIFF_OUT=$(reach --no-input visual diff "$GOLDEN" "$CAPTURED" \
|
||||
--tolerance "$TOLERANCE" \
|
||||
--diff-output "$DIFF_DIR/$scenario-diff.png" \
|
||||
--config "$CONFIG" 2>&1)
|
||||
|
||||
@@ -0,0 +1,11 @@
|
||||
"""The `godot` domain — does the client parse, and does it parse cold.
|
||||
|
||||
Two checks with a deliberate division of labour that the names do not convey,
|
||||
so it is restated here: `cold-parse` only ever sees scripts on the STARTUP path
|
||||
(autoloads plus the main scene chain), which is correct for the registration
|
||||
ORDER bug it was built for and far narrower than its name implies.
|
||||
`parse-sweep` opens every .gd in the project.
|
||||
|
||||
Verified 2026-07-27 by breaking a non-startup UI script and a test file in turn:
|
||||
cold-parse reported clean, exit 0, for both.
|
||||
"""
|
||||
@@ -0,0 +1,73 @@
|
||||
"""Transport for the `godot` domain — args in, delegate, format out."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import typer
|
||||
|
||||
from tooling.core import cli, console
|
||||
from tooling.core.command import command
|
||||
from tooling.core.errors import ReachError
|
||||
from tooling.domains.godot import service
|
||||
from tooling.domains.godot.schemas import ParseResult
|
||||
|
||||
app = cli.domain("godot", "Does the client parse, and does it parse cold.")
|
||||
|
||||
|
||||
@app.callback()
|
||||
def _domain() -> None:
|
||||
"""Keeps `godot` a group (Typer collapses a single-command app)."""
|
||||
|
||||
|
||||
def _verdict(result: ParseResult, name: str, clean: str) -> None:
|
||||
"""Shared rendering — three failure shapes, three different remedies."""
|
||||
if result.engine_failed:
|
||||
raise ReachError(
|
||||
f"{name}: godot itself exited {result.engine_exit} — not a parse verdict\n"
|
||||
+ "\n".join(f" {line}" for line in result.lines),
|
||||
fix="the engine failed rather than the code; check the Godot install "
|
||||
"with `godot --version`, or make setup-godot",
|
||||
exit_code=result.engine_exit,
|
||||
)
|
||||
|
||||
if result.did_not_run:
|
||||
raise ReachError(
|
||||
f"{name}: the sweep did not report completion — no verdict possible\n"
|
||||
+ "\n".join(f" {line}" for line in result.lines),
|
||||
fix="client/tools/parse_sweep.gd did not run to completion; a check "
|
||||
"that examined nothing must not report clean",
|
||||
)
|
||||
|
||||
if result.lines:
|
||||
shown = result.lines[:40]
|
||||
raise ReachError(
|
||||
f"{name}: at least one script does not parse\n"
|
||||
+ "\n".join(f" {line}" for line in shown)
|
||||
+ (f"\n {result.summary}" if result.summary else ""),
|
||||
fix="an unparseable file cannot run — if it is a test suite it did "
|
||||
"not execute, and any pass count reported elsewhere excludes it",
|
||||
)
|
||||
|
||||
console.verdict(clean)
|
||||
|
||||
|
||||
@app.command("parse-sweep")
|
||||
@command
|
||||
def parse_sweep() -> None:
|
||||
"""Open every project .gd and fail on any that will not parse."""
|
||||
result = service.parse_sweep()
|
||||
_verdict(
|
||||
result,
|
||||
"godot-parse-sweep",
|
||||
f"godot-parse-sweep: clean — {result.summary.removeprefix('parse-sweep: ')}",
|
||||
)
|
||||
|
||||
|
||||
@app.command("cold-parse")
|
||||
@command
|
||||
def cold_parse(
|
||||
run_menu: bool = typer.Option(
|
||||
False, "--run-menu", help="Also launch main_menu.tscn briefly, for UI branches."
|
||||
),
|
||||
) -> None:
|
||||
"""Cold-cache startup parse — the class_name/autoload registration race."""
|
||||
_verdict(service.cold_parse(run_menu), "godot-cold-parse", "godot-cold-parse: clean")
|
||||
@@ -0,0 +1,32 @@
|
||||
"""Data shapes for the `godot` domain."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from pydantic import BaseModel, ConfigDict
|
||||
|
||||
|
||||
class ParseResult(BaseModel):
|
||||
"""The outcome of a parse check.
|
||||
|
||||
Three distinct not-ok states, kept apart because they mean different things
|
||||
and only one of them is a verdict about the code:
|
||||
|
||||
- `engine_failed` — godot itself crashed or is missing. NOT a parse verdict,
|
||||
and reporting it as one would blame the tree for a broken toolchain.
|
||||
- `did_not_run` — the sweep produced no completion marker, so it checked
|
||||
nothing. Zero errors from a check that never ran reads as clean, which is
|
||||
the false-green this exists to close.
|
||||
- `lines` non-empty — genuine parse failures.
|
||||
"""
|
||||
|
||||
model_config = ConfigDict(frozen=True)
|
||||
|
||||
lines: list[str] = []
|
||||
summary: str = ""
|
||||
engine_failed: bool = False
|
||||
engine_exit: int = 0
|
||||
did_not_run: bool = False
|
||||
|
||||
@property
|
||||
def ok(self) -> bool:
|
||||
return not (self.lines or self.engine_failed or self.did_not_run)
|
||||
@@ -0,0 +1,136 @@
|
||||
"""Logic for the `godot` domain. Transport-agnostic (D-263).
|
||||
|
||||
Ported from two bash scripts. What was shell — running the engine — stays a
|
||||
guarded exec; what was grep pipelines deciding a verdict is Python now, which
|
||||
is the whole point: these filters encode five separate hard-won lessons and
|
||||
each was a comment in a pipeline nobody could test.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import re
|
||||
|
||||
from tooling.core import config, console, process
|
||||
from tooling.domains.godot.schemas import ParseResult
|
||||
|
||||
# The GDScript half only OPENS files; it makes no verdict, because no Godot API
|
||||
# reports parse failure reliably — one segfaults, one false-positives 150/226,
|
||||
# and plain load() returns non-null for a broken script. The engine's own
|
||||
# stderr is the only honest signal, so the verdict is scraped from it.
|
||||
_SWEEP_RAN = re.compile(r"^parse-sweep: opened", re.MULTILINE)
|
||||
_SWEEP_ERRORS = re.compile(r"Parse Error|Failed to load script")
|
||||
_COLD_ERRORS = re.compile(r"^(SCRIPT )?ERROR|Parse Error|Export type", re.IGNORECASE)
|
||||
|
||||
# Imported assets "fail loading" on a cold tree before the import pass; that is
|
||||
# noise, not a parse failure.
|
||||
_ASSET_NOISE = "Failed loading resource: res://assets"
|
||||
|
||||
# Filtered by cold-parse ONLY. Deliberately NOT filtered by the sweep: that
|
||||
# suppression is why cold-parse stayed silent about a test helper that
|
||||
# genuinely does not parse — the muted class was hiding a real failure.
|
||||
_COLD_ONLY_NOISE = (
|
||||
"Cannot infer the type",
|
||||
)
|
||||
_COLD_NOT_DECLARED = re.compile(
|
||||
r'(Messagepack|LocalBridge|ServerProcess|Constants)" not declared'
|
||||
)
|
||||
|
||||
|
||||
def parse_sweep() -> ParseResult:
|
||||
"""Open every project .gd and report any that will not parse."""
|
||||
result = _godot("-s", "res://tools/parse_sweep.gd")
|
||||
if result.returncode != 0:
|
||||
return ParseResult(
|
||||
engine_failed=True,
|
||||
engine_exit=result.returncode,
|
||||
lines=_tail(result),
|
||||
)
|
||||
|
||||
raw = result.stdout + result.stderr
|
||||
if not _SWEEP_RAN.search(raw):
|
||||
# Without this the sweep could silently stop walking — a renamed script
|
||||
# or a broken loop yields zero error lines, which reads as clean. That
|
||||
# is the same false-green this tool exists to close.
|
||||
return ParseResult(
|
||||
did_not_run=True,
|
||||
lines=_tail(result),
|
||||
)
|
||||
|
||||
summary = next(
|
||||
(line for line in raw.splitlines() if line.startswith("parse-sweep: opened")), ""
|
||||
)
|
||||
errors = [
|
||||
line
|
||||
for line in raw.splitlines()
|
||||
if _SWEEP_ERRORS.search(line) and _ASSET_NOISE not in line
|
||||
]
|
||||
return ParseResult(summary=summary, lines=errors)
|
||||
|
||||
|
||||
def cold_parse(run_menu: bool = False) -> ParseResult:
|
||||
"""Cold-cache startup parse — the class_name/autoload registration race."""
|
||||
client = config.path("client")
|
||||
cache = client / ".godot" / "global_script_class_cache.cfg"
|
||||
cache.unlink(missing_ok=True)
|
||||
|
||||
imported = client / ".godot" / "imported"
|
||||
if not imported.is_dir() or not any(imported.iterdir()):
|
||||
# A truly cold checkout has no import cache, and every imported asset
|
||||
# then "fails loading" during the parse run — a wall of false
|
||||
# positives. Found live in a fresh worktree, 2026-07-13.
|
||||
console.event("no import cache — running one-time import pass", level="warn")
|
||||
seeded = _godot("--import")
|
||||
if seeded.returncode != 0:
|
||||
return ParseResult(
|
||||
engine_failed=True, engine_exit=seeded.returncode, lines=_tail(seeded)
|
||||
)
|
||||
|
||||
result = _godot("--quit")
|
||||
if result.returncode != 0:
|
||||
return ParseResult(
|
||||
engine_failed=True, engine_exit=result.returncode, lines=_tail(result)
|
||||
)
|
||||
|
||||
errors = _cold_filter((result.stdout + result.stderr).splitlines())
|
||||
|
||||
if run_menu:
|
||||
# No exit-code check: `timeout` kills the menu by design, so only the
|
||||
# scraped lines carry signal for this bounded run.
|
||||
menu = process.run(
|
||||
["timeout", "10", "godot", "--path", str(client), "res://scenes/main_menu.tscn"],
|
||||
check=False,
|
||||
missing_fix="install Godot — make setup-godot",
|
||||
)
|
||||
errors += _cold_filter((menu.stdout + menu.stderr).splitlines())
|
||||
|
||||
# Restore a FULL class cache before returning. The cold run re-seeds it only
|
||||
# partially — addon classes are missing, which leaves the gdUnit4 runner
|
||||
# unable to start at all (0 tests in ~350ms; found live when the push gate
|
||||
# ran the suite straight after this check, 2026-07-14). The verdict above is
|
||||
# already decided; this just leaves the tree runnable.
|
||||
_godot("--import")
|
||||
|
||||
return ParseResult(lines=errors)
|
||||
|
||||
|
||||
def _cold_filter(lines: list[str]) -> list[str]:
|
||||
return [
|
||||
line
|
||||
for line in lines
|
||||
if _COLD_ERRORS.search(line)
|
||||
and _ASSET_NOISE not in line
|
||||
and not any(noise in line for noise in _COLD_ONLY_NOISE)
|
||||
and not _COLD_NOT_DECLARED.search(line)
|
||||
]
|
||||
|
||||
|
||||
def _godot(*args: str):
|
||||
return process.run(
|
||||
["godot", "--headless", "--path", str(config.path("client")), *args],
|
||||
check=False,
|
||||
missing_fix="install Godot — make setup-godot",
|
||||
)
|
||||
|
||||
|
||||
def _tail(result, count: int = 20) -> list[str]:
|
||||
return (result.stdout + result.stderr).splitlines()[-count:]
|
||||
@@ -0,0 +1,7 @@
|
||||
"""The `visual` domain — comparing captures, and catching empty ones.
|
||||
|
||||
`blank-check` deserves its place beside `diff`: a capture that is 99% one
|
||||
colour will match any other blank capture forever, so a golden recorded from
|
||||
one is a test that can never fail. `diff` alone cannot see that — two blank
|
||||
images are identical.
|
||||
"""
|
||||
@@ -38,9 +38,10 @@ Usage:
|
||||
Exit: 0 = has content, 1 = blank, 2 = error (unreadable/missing).
|
||||
"""
|
||||
|
||||
import sys
|
||||
from pathlib import Path
|
||||
|
||||
from tooling.core import console
|
||||
|
||||
DEFAULT_MAX_MODAL = 0.85
|
||||
|
||||
|
||||
@@ -49,7 +50,7 @@ def modal_fraction(path: Path) -> tuple[float, int]:
|
||||
try:
|
||||
from PIL import Image
|
||||
except ImportError:
|
||||
print("visual-blank-check: Pillow not available", file=sys.stderr)
|
||||
console.event("visual-blank-check: Pillow not available", level="error")
|
||||
raise SystemExit(2)
|
||||
|
||||
with Image.open(path) as im:
|
||||
@@ -67,39 +68,24 @@ def modal_fraction(path: Path) -> tuple[float, int]:
|
||||
return top / total, len(colors)
|
||||
|
||||
|
||||
def main(argv: list[str]) -> int:
|
||||
args = [a for a in argv if not a.startswith("--")]
|
||||
if not args:
|
||||
print(__doc__, file=sys.stderr)
|
||||
return 2
|
||||
|
||||
max_modal = DEFAULT_MAX_MODAL
|
||||
for i, a in enumerate(argv):
|
||||
if a == "--max-modal" and i + 1 < len(argv):
|
||||
max_modal = float(argv[i + 1])
|
||||
quiet = "--quiet" in argv
|
||||
|
||||
path = Path(args[0])
|
||||
def run(path, max_modal: float = DEFAULT_MAX_MODAL, quiet: bool = False) -> int:
|
||||
"""Fail if a capture is overwhelmingly one colour. Returns 0, 1 or 2."""
|
||||
if not path.is_file():
|
||||
print(f"visual-blank-check: no such file: {path}", file=sys.stderr)
|
||||
console.event(f"visual-blank-check: no such file: {path}", level="error")
|
||||
return 2
|
||||
|
||||
modal, distinct = modal_fraction(path)
|
||||
|
||||
if modal > max_modal:
|
||||
print(
|
||||
console.event(
|
||||
f"BLANK: {path.name} is {modal:.1%} a single colour "
|
||||
f"({distinct} distinct) — the renderer drew nothing but chrome. "
|
||||
f"A golden recorded from this would pass against any other blank "
|
||||
f"capture forever.",
|
||||
file=sys.stderr,
|
||||
level="error",
|
||||
)
|
||||
return 1
|
||||
|
||||
if not quiet:
|
||||
print(f"content: {path.name} modal={modal:.1%} distinct={distinct}")
|
||||
console.event(f"content: {path.name} modal={modal:.1%} distinct={distinct}")
|
||||
return 0
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
raise SystemExit(main(sys.argv[1:]))
|
||||
@@ -13,14 +13,15 @@ Exit codes:
|
||||
2 = size mismatch or fatal error
|
||||
"""
|
||||
|
||||
import argparse
|
||||
import json
|
||||
import struct
|
||||
import sys
|
||||
import zlib
|
||||
from pathlib import Path
|
||||
|
||||
ROOT = Path(__file__).resolve().parent.parent
|
||||
from tooling.core import config, console
|
||||
from tooling.core.errors import ReachError
|
||||
|
||||
ROOT = config.repo_root()
|
||||
DEFAULT_CONFIG = ROOT / "tests" / "visual.json"
|
||||
DEFAULT_TOLERANCE = 5
|
||||
|
||||
@@ -61,8 +62,12 @@ def _read_png_stdlib(path: str) -> tuple[int, int, bytes]:
|
||||
with open(path, "rb") as f:
|
||||
sig = f.read(8)
|
||||
if sig != b"\x89PNG\r\n\x1a\n":
|
||||
print(f"ERROR: {path} is not a valid PNG", file=sys.stderr)
|
||||
sys.exit(2)
|
||||
console.event(f"ERROR: {path} is not a valid PNG", level="error")
|
||||
raise ReachError(
|
||||
"visual-diff: unreadable PNG",
|
||||
fix="the file is not an 8-bit RGBA PNG — re-capture it",
|
||||
exit_code=2,
|
||||
)
|
||||
|
||||
width = height = 0
|
||||
bit_depth = color_type = 0
|
||||
@@ -81,17 +86,25 @@ def _read_png_stdlib(path: str) -> tuple[int, int, bytes]:
|
||||
">IIBB", data[:10]
|
||||
)
|
||||
if color_type != 6:
|
||||
print(
|
||||
console.event(
|
||||
f"ERROR: {path} has color type {color_type}, expected 6 (RGBA)",
|
||||
file=sys.stderr,
|
||||
level="error",
|
||||
)
|
||||
sys.exit(2)
|
||||
raise ReachError(
|
||||
"visual-diff: unreadable PNG",
|
||||
fix="the file is not an 8-bit RGBA PNG — re-capture it",
|
||||
exit_code=2,
|
||||
)
|
||||
if bit_depth != 8:
|
||||
print(
|
||||
console.event(
|
||||
f"ERROR: {path} has bit depth {bit_depth}, expected 8",
|
||||
file=sys.stderr,
|
||||
level="error",
|
||||
)
|
||||
sys.exit(2)
|
||||
raise ReachError(
|
||||
"visual-diff: unreadable PNG",
|
||||
fix="the file is not an 8-bit RGBA PNG — re-capture it",
|
||||
exit_code=2,
|
||||
)
|
||||
elif chunk_type == b"IDAT":
|
||||
idat_chunks.append(data)
|
||||
elif chunk_type == b"IEND":
|
||||
@@ -132,11 +145,15 @@ def _read_png_stdlib(path: str) -> tuple[int, int, bytes]:
|
||||
elif filter_type == 4: # Paeth
|
||||
val = (cur + _paeth(a, b, c)) & 0xFF
|
||||
else:
|
||||
print(
|
||||
console.event(
|
||||
f"ERROR: unknown PNG filter type {filter_type} at row {y}",
|
||||
file=sys.stderr,
|
||||
level="error",
|
||||
)
|
||||
sys.exit(2)
|
||||
raise ReachError(
|
||||
"visual-diff: unreadable PNG",
|
||||
fix="the file is not an 8-bit RGBA PNG — re-capture it",
|
||||
exit_code=2,
|
||||
)
|
||||
|
||||
pixels[row_start + x] = val
|
||||
|
||||
@@ -262,66 +279,50 @@ def load_config(config_path: Path | None) -> dict:
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def main() -> int:
|
||||
parser = argparse.ArgumentParser(
|
||||
description="Pixel-level visual diff for golden image comparison."
|
||||
)
|
||||
parser.add_argument("expected", help="Path to golden PNG")
|
||||
parser.add_argument("actual", help="Path to captured PNG")
|
||||
parser.add_argument(
|
||||
"--tolerance",
|
||||
type=int,
|
||||
default=None,
|
||||
help="Per-channel pixel tolerance (default: from config or 5)",
|
||||
)
|
||||
parser.add_argument(
|
||||
"--max-diff-pct",
|
||||
type=float,
|
||||
default=None,
|
||||
help="Max allowed diff percentage (default: from config or 0.0)",
|
||||
)
|
||||
parser.add_argument(
|
||||
"--diff-output",
|
||||
default=None,
|
||||
help="Path to write diff PNG highlighting changed pixels",
|
||||
)
|
||||
parser.add_argument(
|
||||
"--config",
|
||||
default=None,
|
||||
help="Path to tests/visual.json (default: auto-detect)",
|
||||
)
|
||||
args = parser.parse_args()
|
||||
def run(
|
||||
expected: str,
|
||||
actual: str,
|
||||
tolerance: int | None = None,
|
||||
max_diff_pct: float | None = None,
|
||||
diff_output: str | None = None,
|
||||
config_path_arg: str | None = None,
|
||||
) -> int:
|
||||
"""Pixel-level visual diff. Returns 0 pass, 1 fail, 2 unusable input.
|
||||
|
||||
The argparse parser that used to live here is gone: the router declares the
|
||||
options, and a parser inside a service would be a second transport layer
|
||||
(D-263)."""
|
||||
|
||||
# Resolve settings: CLI > config > fallback
|
||||
config_path = Path(args.config) if args.config else None
|
||||
config_path = Path(config_path_arg) if config_path_arg else None
|
||||
cfg = load_config(config_path)
|
||||
tolerance = args.tolerance if args.tolerance is not None else cfg["tolerance"]
|
||||
max_diff_pct = args.max_diff_pct if args.max_diff_pct is not None else cfg["max_diff_pct"]
|
||||
tolerance = tolerance if tolerance is not None else cfg["tolerance"]
|
||||
max_diff_pct = max_diff_pct if max_diff_pct is not None else cfg["max_diff_pct"]
|
||||
|
||||
# Read images
|
||||
try:
|
||||
ew, eh, epx = read_png(args.expected)
|
||||
ew, eh, epx = read_png(expected)
|
||||
except FileNotFoundError:
|
||||
print(f"ERROR: expected image not found: {args.expected}", file=sys.stderr)
|
||||
console.event(f"ERROR: expected image not found: {expected}", level="error")
|
||||
return 2
|
||||
except Exception as exc:
|
||||
print(f"ERROR: failed to read expected image: {exc}", file=sys.stderr)
|
||||
console.event(f"ERROR: failed to read expected image: {exc}", level="error")
|
||||
return 2
|
||||
|
||||
try:
|
||||
aw, ah, apx = read_png(args.actual)
|
||||
aw, ah, apx = read_png(actual)
|
||||
except FileNotFoundError:
|
||||
print(f"ERROR: actual image not found: {args.actual}", file=sys.stderr)
|
||||
console.event(f"ERROR: actual image not found: {actual}", level="error")
|
||||
return 2
|
||||
except Exception as exc:
|
||||
print(f"ERROR: failed to read actual image: {exc}", file=sys.stderr)
|
||||
console.event(f"ERROR: failed to read actual image: {exc}", level="error")
|
||||
return 2
|
||||
|
||||
# Size check
|
||||
if ew != aw or eh != ah:
|
||||
print(
|
||||
console.event(
|
||||
f"ERROR: size mismatch — expected {ew}x{eh}, actual {aw}x{ah}",
|
||||
file=sys.stderr,
|
||||
level="error",
|
||||
)
|
||||
return 2
|
||||
|
||||
@@ -330,23 +331,20 @@ def main() -> int:
|
||||
total = ew * eh
|
||||
|
||||
if diff_count == 0:
|
||||
print(f"PASS: images match ({ew}x{eh})")
|
||||
console.event(f"PASS: images match ({ew}x{eh})")
|
||||
return 0
|
||||
|
||||
pct = diff_count / total * 100
|
||||
|
||||
if pct <= max_diff_pct:
|
||||
print(f"PASS: {diff_count} of {total} pixels differ ({pct:.1f}%, within {max_diff_pct}% threshold)")
|
||||
console.event(f"PASS: {diff_count} of {total} pixels differ ({pct:.1f}%, within {max_diff_pct}% threshold)")
|
||||
return 0
|
||||
|
||||
print(f"FAIL: {diff_count} of {total} pixels differ ({pct:.1f}%)")
|
||||
console.event(f"FAIL: {diff_count} of {total} pixels differ ({pct:.1f}%)")
|
||||
|
||||
if args.diff_output and diff_buf:
|
||||
Path(args.diff_output).parent.mkdir(parents=True, exist_ok=True)
|
||||
write_png(args.diff_output, ew, eh, diff_buf)
|
||||
if diff_output and diff_buf:
|
||||
Path(diff_output).parent.mkdir(parents=True, exist_ok=True)
|
||||
write_png(diff_output, ew, eh, diff_buf)
|
||||
|
||||
return 1
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
sys.exit(main())
|
||||
@@ -0,0 +1,89 @@
|
||||
"""Transport for the `visual` domain — args in, delegate, format out."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from pathlib import Path
|
||||
|
||||
import typer
|
||||
|
||||
from tooling.core import cli, console
|
||||
from tooling.core.command import command
|
||||
from tooling.core.errors import ReachError
|
||||
from tooling.domains.visual import blank_check as blank_module
|
||||
from tooling.domains.visual import diff as diff_module
|
||||
from tooling.domains.visual import thumbnail as thumbnail_module
|
||||
|
||||
app = cli.domain("visual", "Compare captures against goldens, and catch blank ones.")
|
||||
|
||||
|
||||
@app.callback()
|
||||
def _domain() -> None:
|
||||
"""Keeps `visual` a group (Typer collapses a single-command app)."""
|
||||
|
||||
|
||||
@app.command("diff")
|
||||
@command
|
||||
def diff(
|
||||
expected: str = typer.Argument(..., help="Path to the golden PNG."),
|
||||
actual: str = typer.Argument(..., help="Path to the captured PNG."),
|
||||
tolerance: int = typer.Option(None, "--tolerance", help="Per-channel pixel tolerance."),
|
||||
max_diff_pct: float = typer.Option(
|
||||
None, "--max-diff-pct", help="Maximum allowed differing percentage."
|
||||
),
|
||||
diff_output: str = typer.Option(
|
||||
None, "--diff-output", help="Write a PNG highlighting the changed pixels."
|
||||
),
|
||||
config: str = typer.Option(None, "--config", help="Path to tests/visual.json."),
|
||||
) -> None:
|
||||
"""Pixel-level comparison of a capture against its golden."""
|
||||
code = diff_module.run(expected, actual, tolerance, max_diff_pct, diff_output, config)
|
||||
if code == 0:
|
||||
return
|
||||
raise ReachError(
|
||||
f"visual-diff: {expected} and {actual} differ beyond the threshold",
|
||||
fix="inspect the diff PNG with --diff-output, or update the golden with "
|
||||
"make visual-update if the change is intended",
|
||||
exit_code=code,
|
||||
)
|
||||
|
||||
|
||||
@app.command("blank-check")
|
||||
@command
|
||||
def blank_check(
|
||||
path: Path = typer.Argument(..., help="The capture to inspect."),
|
||||
max_modal: float = typer.Option(
|
||||
blank_module.DEFAULT_MAX_MODAL,
|
||||
"--max-modal",
|
||||
help="Fail above this single-colour fraction.",
|
||||
),
|
||||
quiet: bool = typer.Option(False, "--quiet", help="Say nothing when the frame has content."),
|
||||
) -> None:
|
||||
"""Fail if a capture is overwhelmingly one colour."""
|
||||
code = blank_module.run(path, max_modal, quiet)
|
||||
if code == 0:
|
||||
return
|
||||
raise ReachError(
|
||||
f"visual-blank-check: {path} has no content",
|
||||
fix="the renderer drew nothing but chrome — check the scenario actually "
|
||||
"loaded before recording a golden from this",
|
||||
exit_code=code,
|
||||
)
|
||||
|
||||
|
||||
@app.command("thumbnail")
|
||||
@command
|
||||
def thumbnail(
|
||||
target: Path = typer.Argument(..., help="Capture directory, or an image in crop mode."),
|
||||
crop: str = typer.Option(None, "--crop", help="Extract this named region instead."),
|
||||
config: Path = typer.Option(None, "--config", help="Config JSON path."),
|
||||
) -> None:
|
||||
"""Build a contact sheet from a flow capture, or crop a named region."""
|
||||
code = thumbnail_module.run(target, crop, config)
|
||||
if code == 0:
|
||||
console.verdict(f"visual-thumbnail: OK — {target}")
|
||||
return
|
||||
raise ReachError(
|
||||
f"visual-thumbnail: failed for {target}",
|
||||
fix="check the target exists and the config names the region you asked for",
|
||||
exit_code=code,
|
||||
)
|
||||
@@ -18,18 +18,22 @@ Config: reads thumbnail dimensions, columns, and crop regions from
|
||||
tests/visual.json (auto-detected from script location, or --config).
|
||||
"""
|
||||
|
||||
import argparse
|
||||
import json
|
||||
import sys
|
||||
from pathlib import Path
|
||||
|
||||
from tooling.core import config, console
|
||||
from tooling.core.errors import ReachError
|
||||
|
||||
try:
|
||||
from PIL import Image, ImageDraw
|
||||
except ImportError:
|
||||
print("visual-thumbnail requires Pillow: pip install Pillow", file=sys.stderr)
|
||||
sys.exit(1)
|
||||
console.event("visual-thumbnail requires Pillow: pip install Pillow", level="error")
|
||||
raise ReachError(
|
||||
"visual-thumbnail: config not found",
|
||||
fix="pass --config, or restore tests/visual.json",
|
||||
)
|
||||
|
||||
ROOT = Path(__file__).resolve().parent.parent
|
||||
ROOT = config.repo_root()
|
||||
DEFAULT_CONFIG = ROOT / "tests" / "visual.json"
|
||||
|
||||
# Fallbacks when config keys are missing
|
||||
@@ -42,8 +46,8 @@ LABEL_HEIGHT = 24 # pixels reserved below each thumbnail for text
|
||||
def load_config(config_path: Path) -> dict:
|
||||
"""Load configuration from JSON file."""
|
||||
if not config_path.exists():
|
||||
print(f"Config not found: {config_path}", file=sys.stderr)
|
||||
print("Continuing with built-in defaults.", file=sys.stderr)
|
||||
console.event(f"Config not found: {config_path}", level="error")
|
||||
console.event("Continuing with built-in defaults.", level="error")
|
||||
return {}
|
||||
with open(config_path) as f:
|
||||
return json.load(f)
|
||||
@@ -81,9 +85,9 @@ def detect_flow(directory: Path) -> str | None:
|
||||
stem = manifests[0].stem
|
||||
return stem.removesuffix("_manifest")
|
||||
if len(manifests) > 1:
|
||||
print(f"Multiple manifests found in {directory}:", file=sys.stderr)
|
||||
console.event(f"Multiple manifests found in {directory}:", level="error")
|
||||
for m in manifests:
|
||||
print(f" {m.name}", file=sys.stderr)
|
||||
console.event(f" {m.name}", level="error")
|
||||
return None
|
||||
return None
|
||||
|
||||
@@ -92,18 +96,18 @@ def grid_mode(directory: Path, config: dict) -> int:
|
||||
"""Generate a contact sheet from flow captures."""
|
||||
directory = directory.resolve()
|
||||
if not directory.is_dir():
|
||||
print(f"Not a directory: {directory}", file=sys.stderr)
|
||||
console.event(f"Not a directory: {directory}", level="error")
|
||||
return 1
|
||||
|
||||
flow = detect_flow(directory)
|
||||
if flow is None:
|
||||
print(f"No manifest found in {directory}. Expected {{flow}}_manifest.txt", file=sys.stderr)
|
||||
console.event(f"No manifest found in {directory}. Expected {{flow}}_manifest.txt", level="error")
|
||||
return 1
|
||||
|
||||
manifest_path = directory / f"{flow}_manifest.txt"
|
||||
entries = parse_manifest(manifest_path)
|
||||
if not entries:
|
||||
print(f"Empty manifest: {manifest_path}", file=sys.stderr)
|
||||
console.event(f"Empty manifest: {manifest_path}", level="error")
|
||||
return 1
|
||||
|
||||
# Read thumbnail config
|
||||
@@ -123,7 +127,7 @@ def grid_mode(directory: Path, config: dict) -> int:
|
||||
for idx, entry in enumerate(entries):
|
||||
frame_file = directory / f"{flow}_{entry['frame']}.png"
|
||||
if not frame_file.exists():
|
||||
print(f" Missing frame: {frame_file.name}", file=sys.stderr)
|
||||
console.event(f" Missing frame: {frame_file.name}", level="error")
|
||||
continue
|
||||
|
||||
img = Image.open(frame_file)
|
||||
@@ -148,7 +152,7 @@ def grid_mode(directory: Path, config: dict) -> int:
|
||||
|
||||
output_path = directory / f"{flow}_sheet.png"
|
||||
sheet.save(output_path)
|
||||
print(f"Sheet: {output_path}")
|
||||
console.event(f"Sheet: {output_path}")
|
||||
return 0
|
||||
|
||||
|
||||
@@ -156,19 +160,19 @@ def crop_mode(region_name: str, image_path: Path, config: dict) -> int:
|
||||
"""Extract a named crop region from an image at 1:1 scale."""
|
||||
image_path = image_path.resolve()
|
||||
if not image_path.exists():
|
||||
print(f"Image not found: {image_path}", file=sys.stderr)
|
||||
console.event(f"Image not found: {image_path}", level="error")
|
||||
return 1
|
||||
|
||||
crops = config.get("crops", {})
|
||||
if region_name not in crops:
|
||||
available = ", ".join(sorted(crops.keys())) if crops else "(none)"
|
||||
print(f"Unknown crop region: {region_name}", file=sys.stderr)
|
||||
print(f"Available regions: {available}", file=sys.stderr)
|
||||
console.event(f"Unknown crop region: {region_name}", level="error")
|
||||
console.event(f"Available regions: {available}", level="error")
|
||||
return 1
|
||||
|
||||
coords = crops[region_name]
|
||||
if not isinstance(coords, list) or len(coords) != 4:
|
||||
print(f"Invalid crop coords for '{region_name}': expected [x, y, w, h]", file=sys.stderr)
|
||||
console.event(f"Invalid crop coords for '{region_name}': expected [x, y, w, h]", level="error")
|
||||
return 1
|
||||
|
||||
x, y, w, h = coords
|
||||
@@ -179,39 +183,19 @@ def crop_mode(region_name: str, image_path: Path, config: dict) -> int:
|
||||
suffix = image_path.suffix
|
||||
output_path = image_path.parent / f"{stem}_crop_{region_name}{suffix}"
|
||||
cropped.save(output_path)
|
||||
print(output_path)
|
||||
console.event(output_path)
|
||||
return 0
|
||||
|
||||
|
||||
def main() -> int:
|
||||
parser = argparse.ArgumentParser(
|
||||
description="Contact sheet and crop tool for visual QA flow captures.",
|
||||
)
|
||||
parser.add_argument(
|
||||
"--config", type=Path, default=DEFAULT_CONFIG,
|
||||
help=f"Config JSON path (default: {DEFAULT_CONFIG.relative_to(ROOT)})",
|
||||
)
|
||||
def run(target, crop: str | None = None, config_path=None) -> int:
|
||||
"""Contact sheet (grid mode) or named-region crop, depending on `crop`.
|
||||
|
||||
# Crop mode
|
||||
parser.add_argument(
|
||||
"--crop", metavar="REGION",
|
||||
help="Crop mode: extract named region from IMAGE",
|
||||
)
|
||||
|
||||
# Positional: DIR (grid mode) or IMAGE (crop mode)
|
||||
parser.add_argument(
|
||||
"target", type=Path,
|
||||
help="Directory of flow captures (grid mode) or image file (crop mode)",
|
||||
)
|
||||
|
||||
args = parser.parse_args()
|
||||
config = load_config(args.config)
|
||||
|
||||
if args.crop:
|
||||
return crop_mode(args.crop, args.target, config)
|
||||
else:
|
||||
return grid_mode(args.target, config)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
sys.exit(main())
|
||||
Two modes behind one entry, as the original had them. The router exposes
|
||||
them as one verb with a flag rather than two, because the second argument
|
||||
changes what `target` MEANS — a directory in grid mode, a file in crop
|
||||
mode — and two verbs would each have to re-explain that.
|
||||
"""
|
||||
config = load_config(config_path or DEFAULT_CONFIG)
|
||||
if crop:
|
||||
return crop_mode(crop, target, config)
|
||||
return grid_mode(target, config)
|
||||
@@ -55,6 +55,14 @@ DOMAINS: dict[str, tuple[str, str]] = {
|
||||
"tooling.domains.validate.router:app",
|
||||
"Content, checklists and RON against their schemas",
|
||||
),
|
||||
"godot": (
|
||||
"tooling.domains.godot.router:app",
|
||||
"Does the client parse, and does it parse cold",
|
||||
),
|
||||
"visual": (
|
||||
"tooling.domains.visual.router:app",
|
||||
"Compare captures against goldens, and catch blank ones",
|
||||
),
|
||||
"jobs": (
|
||||
"tooling.domains.jobs.router:app",
|
||||
"Detached runs — status, logs and outcomes",
|
||||
|
||||
Reference in New Issue
Block a user