SEP-1553: Migrate alters onto declarative spec path

[yyyyyyyan]

Summary

• Migrate the alters app onto the framework's canonical (form, resolved) -> RunCommandSpec + assemble_envelope path (matching the already-migrated checksums app): annotate AltersCreate with additive ArgFormat markers, replace the hand-rolled optional_args/flag_args dicts with build_command_args, and drop the hand-assembled TaskWrite. build_alters_task now threads the resolved ResolvedEntities through the spec builder, so the resolved executor host drives the envelope target instead of being discarded.
• Add derive_arg_parser_from_model to build the reverse (args→form) parser's value/flag mappings from a model's ArgFormat markers, and switch both the alters and checksums reverse parsers onto it — deleting the four hand-maintained mapping dicts so the markers are the single source of truth in both directions.
• Share the ArgFormat template resolver by relocating find_arg_format/resolve_arg_template into form_dsl/markers.py, and document the two blessed spec signatures in the framework spec module docstring.
• The emitted TaskWrite payload stays byte-for-byte identical — locked by a new alters payload golden captured from the pre-refactor baseline and asserted across three legs (pure spec path, build_alters_task, and the legacy flat-form path). The reverse parse round-trips unchanged, guarded by the existing round-trip tests plus a new derived-dict-equality assertion.
• Harden validate_arg_formats to reject a value-arg template whose ${value} is not in the terminal =${value} position — the only shape derive_arg_parser_from_model's reverse parse round-trips — so the forward (render_value_arg, which supports ${value} anywhere) and reverse directions cannot desync on a mid-token placeholder. No current template is affected; the guard fails loud at app construction alongside the existing placeholder-name and flag-on-bool checks.

Deviations (all intentional)

• alter, progress, and pre_checks_mysql_config_file intentionally keep no ArgFormat marker — they render in the app-specific prefix/suffix, not through build_command_args. build_command_args emits all value args before all flags and truthy-gates them, but progress must render after the flags, alter before the DSN positional, and --defaults-file is suppressed by an exact ~/.my.cnf sentinel compare (not truthiness). This mirrors how checksums keeps recursion_method/databases/tables outside ArgFormat.
• The checksums reverse-parser derive call supplies --recursion-method=, --databases=, and --tables= as extra_arg_mappings. databases/tables carry SchemaRef/TableRef (not ArgFormat) and are emitted specially by build_checksums_command_args from resolved refs, so they must be reverse extras. The derived-dict-equality test pins the derived mappings to the deleted dicts byte-for-byte.

Tested

Byte-identical internal refactor — verify no behavior change:

• Create an alters (pt-osc) task via POST /api/apps/alters/; confirm 2xx and that the generated parent run-command meta.args matches a task created for the same inputs before this change.
• Update an alters task via PUT /api/apps/alters/{task_name}; confirm 2xx and an unchanged payload.
• Create an alters task via the legacy Jinja form (POST /alters/); confirm the generated command is identical to the JSON path.
• Open the alters edit form for an existing task; confirm the reverse-parsed fields prefill correctly (recursion method, flags, value args, dsn table).
• Create a checksums task and open its edit form; confirm reverse-parse prefill is unchanged.
• GET /api/apps/alters/schema returns the same schema as before.

Checklist

• New/modified functions have type hints and rST docstrings
• New tests added for new features or bug fixes
• Database migrations generated if models changed (N/A — no DB table model changes; only Pydantic form-model annotations)
• User-facing changes documented (N/A — pure internal refactor, no user-facing change)
• Configuration changes documented (N/A — no config changes)

[Copilot]

Pull request overview

Migrates the alters app onto the framework’s canonical declarative spec/envelope path and removes hand-maintained CLI arg mapping dictionaries by deriving reverse-parser mappings from ArgFormat markers (shared with checksums), with payload identity guarded by new golden snapshot tests.

Changes:

• Refactor alters to emit a RunCommandSpec and rely on assemble_envelope for uniform connectivity/target meta stamping.
• Add derive_arg_parser_from_model() and switch alters/checksums reverse parsing to use it, removing duplicated mapping dicts.
• Add a committed alters payload golden + matrix tests to assert byte-identical envelopes across spec/task/legacy-form paths.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
tests/app/sep/snapshots/payload/alters__payload.json	Adds golden payload matrix snapshot for alters envelopes.
tests/app/sep/apps/framework/test_pt_toolkit.py	Adds unit tests for derive_arg_parser_from_model and freezes prior mapping dict expectations.
tests/app/sep/apps/alters/test_spec.py	Updates alters spec tests to validate RunCommandSpec output and argument ordering/quoting behavior.
tests/app/sep/apps/alters/test_payload_snapshot.py	Adds snapshot tests ensuring spec/task/legacy paths reproduce the golden payload matrix.
app/sep/apps/framework/spec.py	Documents blessed spec signatures; relocates ArgFormat helper logic to markers module and uses it in arg building/validation.
app/sep/apps/framework/form_dsl/pt_toolkit.py	Introduces derive_arg_parser_from_model for reverse parser mapping derivation.
app/sep/apps/framework/form_dsl/markers.py	Adds shared find_arg_format / resolve_arg_template helpers and exports them.
app/sep/apps/framework/form_dsl/init.py	Re-exports the new marker helpers and derive_arg_parser_from_model.
app/sep/apps/checksums/deps.py	Switches checksums reverse-parser mappings to derived mappings.
app/sep/apps/alters/spec.py	Refactors alters spec builder to return RunCommandSpec and use build_command_args.
app/sep/apps/alters/models.py	Annotates alters form fields with additive ArgFormat markers for declarative arg rendering/parsing.
app/sep/apps/alters/deps.py	Updates task builder to resolve refs then use assemble_envelope(build_alters_spec(...)); switches reverse-parser mappings to derived mappings.

[yyyyyyyan]

Automated QA — PASS

Verified the Tested items against a fresh instance on the PR head, cross-checked against a base-branch (origin/main) instance and the committed pre-refactor payload snapshot:

• POST /api/apps/alters/ returns 201 and the generated pt-osc command is byte-identical to the pre-refactor baseline across all ten snapshot input shapes — value-arg ordering, flag ordering, DSN framing, dsn-recursion split, shlex quoting, extra-args tokens, --defaults-file prefix, --progress suffix positioning, and local port elision.

  

• PUT /api/apps/alters/{task_name} returns 200 with an unchanged payload — re-submitting identical inputs yields the same meta.args.

  

• The legacy Jinja form (POST /alters/) produces a command identical to the JSON path — a real form submit routed through the same builder; the generated args match the JSON-path task byte-for-byte (only the executor target differs, reflecting the selected host).

  

• The alters edit form reverse-parses every field correctly — recursion method, DSN table, all five flags, and all nine value args prefill from the stored command, byte-identical to the base branch across 32 form fields.

  

  

• The checksums edit form reverse-parses every field correctly — the four flags, value args, and databases/tables prefill byte-identical to the base branch across 22 form fields.

  

• GET /api/apps/alters/schema returns 200 with a schema identical to the base branch — 10088 bytes, byte-for-byte unchanged.

  

Observations — pre-existing, out of scope

• On the alters edit form, a non-default --defaults-file=<path> round-trips into the Extra Args field rather than the MySQL-defaults field. This is present on the base branch too (verified against origin/main), so it predates this change — the reverse parser in app/sep/apps/alters/deps.py (parse_alters_task_args) has no mapping for --defaults-file=, so the token is collected into extra args.

[github-actions]

Coverage report

Click to see where and how coverage changed

File	Statements	Missing	Coverage	Coverage (new stmts)	Lines missing
app/sep/apps/alters
deps.py					214
models.py
spec.py
app/sep/apps/checksums
deps.py
app/sep/apps/framework
spec.py
app/sep/apps/framework/form_dsl
markers.py					181
pt_toolkit.py
app/sep/sync/syncers
pmm.py
app/sep/sync/syncers/mysql
syncer.py
Project Total

This report was generated by python-coverage-comment-action