Skip to content

Clarify scalar rewrite and config command docstrings - #47

Closed
coderabbitai[bot] wants to merge 5 commits into
mainfrom
coderabbit/changes/e4505e06
Closed

coderabbitai[bot] wants to merge 5 commits into
mainfrom
coderabbit/changes/e4505e06

Conversation

@coderabbitai

@coderabbitai coderabbitai Bot commented Sep 23, 2026

Copy link
Copy Markdown
Contributor

Clarify that scalar rewrites apply only to legacy metadata, preserve surrounding formatting, insert values without escaping, and can fail for unsupported or invalid metadata. Expand config get and validate docstrings to document read and parse errors.

Documentation-only changes; runtime behavior is unchanged. Validation was not run.

View coding task

hyperpolymath and others added 5 commits September 23, 2026 01:11
Phase 2 of #40. Phase 1 shipped the compat reader; this converts the
emitter, so `mint` now writes `@launcher-deed` instead of the retired
`@a2ml-metadata` form. The compliance data is byte-identical either
way — it is purely a dialect change — which is what makes the strongest
available test possible: the new emitter's flattened output is asserted
EQUAL to the committed pre-phase fixture's, field for field.

templates/launcher.sh.tera
  The embedded block becomes a `praxis-deed` form, per the owner's
  ruling on the doc-head. Verified to contain no literal tab:
  `deed::parse` rejects a tab anywhere in the document, checked on the
  raw text before lexing, so a tab surviving into a value would not
  corrupt one field — it would make the whole block unparseable.

crates/launcher-common/src/template.rs
  New `deedstr` Tera filter, the exact inverse of `deed.rs`'s
  `lex_string` and deliberately no stricter: the grammar admits exactly
  four escapes (\" \\ \n \t) and rejects other raw control characters
  below U+0020. U+007F is therefore ACCEPTED, because its consumer
  accepts it — a filter refusing it would ask a different question than
  the parser it feeds. CR has no legal spelling at all, so it is
  reported as an error rather than silently dropped: refusing to mint
  beats minting a launcher whose own metadata cannot be read back.

  Registered as a filter rather than pre-escaping the context so the
  escape applies at exactly the emission sites and every other
  interpolation in the script keeps its raw value. Tera autoescaping is
  HTML-shaped and does not fire for `.sh` regardless.

crates/launcher/src/cmd_config.rs
  Dialect-naming strings corrected — the "no @a2ml-metadata block
  found" message named only the retired form while the reader accepts
  both. Module doc records that `set` refuses a DEED block by design.

Three mutants killed, per the rule that a passing suite proves nothing
until a mutant dies:
  A  `deed_escape` made the identity function  -> 4 tests red
  B  emitter drops one compliance standard     -> 1 test red (the
     paired control alone, proving it checks the DATA and not merely
     that two things parsed)
  C  emitter reverts to the legacy markers      -> 4 tests red
Clean restore re-run: 72/72, so every red above is attributable to the
mutant and not to ambient state.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WPSJ7fBhVAMcpSffCBWUDo
Signed-off-by: Jonathan D.A. Jewell <6759885+hyperpolymath@users.noreply.github.com>
The mirror of `minted-2026-09-22_stapeln-launcher.sh`, and it guards the
other direction. That fixture proves a launcher minted BEFORE the
dialect change still reads. This one will prove a launcher minted
DURING it still reads after some later tightening of the DEED grammar:
`deed::parse` is shared with the rest of the estate and will keep
moving, while the launchers this emitter has already written will not.
Without a committed artefact there is nothing to notice they had been
stranded.

Captured from the real emitter, not typed by hand.

⭐ Mutant D — the emitter's `:generator` string changed — was run
specifically to check this test is not decorative, and it found
something worth recording:

  a_launcher_minted_by_phase_two_reads_...   FAILED  (killed)
  a_launcher_minted_before_phase_two_...     FAILED  (killed)
  both_dialects_agree_on_what_todays_...     ok      <-- BLIND

`both_dialects_agree` builds its legacy leg FROM the deed the emitter
just produced, so under a uniform value change both legs move together
and the equality still holds. It is structurally incapable of catching
that class of change — which is not a defect in it (it exists to prove
cross-dialect agreement, and it does) but it does mean the committed
fixtures are the only thing anchoring the emitter to a known-good
value set. That is the argument for this file existing.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WPSJ7fBhVAMcpSffCBWUDo
Signed-off-by: Jonathan D.A. Jewell <6759885+hyperpolymath@users.noreply.github.com>
@coderabbitai

coderabbitai Bot commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor Author

Important

Review skipped

This PR was authored by the user configured for CodeRabbit reviews. CodeRabbit does not review PRs authored by this user. It's recommended to use a dedicated user account to post CodeRabbit review feedback.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Essentials

Run ID: af197f06-8166-4b04-94be-5798dc1125af

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Comment @coderabbitai help to get the list of available commands.

Base automatically changed from feat/40-phase2-deed-emitter to main September 23, 2026 08:43
@hyperpolymath

Copy link
Copy Markdown
Owner

Closing this as superseded, with nothing discarded — here is the accounting.

Where each of the five commits went

Four of the five landed on main via the #46 squash (552ecf4). The fifth,
4cac602, carried content that was still absent from main, so it has been
cherry-picked with -x onto a fresh branch off 552ecf4 and opened as
#50.

Authorship is preserved: the carried commit is still authored by
coderabbitai[bot] (f1065a77), with this session as committer only.

Why a cherry-pick rather than a rebase or a force-push

git rebase origin/main would not have skipped the four already-landed
commits — their patch-ids do not match a squash commit, so it would have
replayed all five and then needed a force-push over
coderabbit/changes/e4505e06. git cherry-pick -x instead carries exactly the
one unique commit, records the provenance line, preserves the bot's authorship,
and touches nobody else's ref.

Verification done before carrying it

The commit is documentation-only, and every claim in its new doc comments was
checked against main at 552ecf4 rather than taken on trust — all five
documented error paths of rewrite_scalar resolve to real bail! sites, and
the "inserted verbatim without escaping" claim to rewritten.push_str(new_value).
cargo test --workspace is 84 passed / 0 failed.

No content from this PR has been dropped.

🤖 Generated with Claude Code

https://claude.ai/code/session_01WPSJ7fBhVAMcpSffCBWUDo

hyperpolymath added a commit that referenced this pull request Sep 23, 2026
Carries forward the one commit of #47 that is **not** already on `main`.

## Why this exists

Four of #47's five commits landed via the #46 squash (`552ecf4`). Only
`4cac602` carried content that is still absent from `main`, so rather
than
force-push over CodeRabbit's branch or discard the work, that single
commit is
cherry-picked here with `-x`. **Authorship is preserved** — the commit
is still
authored by `coderabbitai[bot]`; this session signs only as committer.

## What it changes

Documentation only. No behaviour change, no test change. Two files, +12
-5.

Every claim in the new doc comments was **verified against `main` at
`552ecf4`**
before being carried, rather than taken on the original PR's word:

| documented error path | verified against |
|---|---|
| refuses the read-only DEED dialect | `if block.is_deed() { bail! }` |
| `new_value` inserted verbatim, no escaping |
`rewritten.push_str(new_value)` |
| key absent, or names a list | the two `bail!`s above the scan |
| parsed but not safely locatable | `if !patched { bail! }` |

The pre-existing docstring named only two of those five error paths, so
this is
a **correctness fix to the documentation**, not a rewording.

## Verification

- `cargo test --workspace` — **84 passed, 0 failed** (73 unit + 5
`deed_corpus`
  + 6 `round_trip`).
- Subject line trimmed to 64 chars; the bot's original was 73, one over
the
  estate's 72-char threshold.
- Commit is signed (`%G?` = `G`).

Closes #47.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

https://claude.ai/code/session_01WPSJ7fBhVAMcpSffCBWUDo

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
@hyperpolymath
hyperpolymath deleted the coderabbit/changes/e4505e06 branch September 23, 2026 09:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant