test(frontmatter): consolidate set_frontmatter_status tests

[pbean]

Part 3 of #357 — the last one. Parts 1 (line endings, #364) and 2 (inline comments, #365) are merged; both were confirmed on main before this branch was cut (frontmatter.py reads read_bytes().decode, _replace_value carries a trailing comment).

Tests only. Zero src/ changes — src/bmad_loop/frontmatter.py is byte-identical to main (verified by diff against a pre-edit copy after the ablation below).

set_frontmatter_status's original tests stayed in tests/test_resolve.py through parts 1 and 2 on purpose: moving them then would have hidden those diffs behind a rename. AGENTS.md — flat tests/ mirrors src modules by name — puts them in tests/test_frontmatter.py, next to the module that defines them. set_frontmatter_field's tests stay in test_resolve.py: that function lives in verify.py, so moving it is a different change.

What happened to each of the four

Judged against the destination file's actual current content, not against the issue's description of it.

test_resolve.py test	Disposition
..._idempotent	Deleted, not moved. test_flipping_to_the_status_already_there_returns_false_and_does_not_write already covers it, and covers it harder: it asserts st_mtime_ns is unchanged, so a no-op write fails it. The resolve copy only checked the return value.
..._no_frontmatter	Deleted, not moved. test_no_frontmatter_block_returns_false_with_the_bytes_unchanged is the same case plus a byte assertion.
..._replaces	Folded, to leave one test rather than two near-twins. Its three substring asserts (owner:, title:, <frozen-after-approval>) are strictly weaker than the destination's byte-exact whole-file comparison — but its fixture carried one thing _PLAIN did not: a <frozen-after-approval> body block. That block moved into _PLAIN, so the byte-exact assertion now covers it (and the CRLF / CR-only / idempotence tests inherit the richer fixture).
..._preserves_triple_dash_in_value	Moved, then strengthened (see below). Nothing in the destination names this property. test_the_verified_edit_is_never_a_yaml_round_trip happens to carry a ----bearing title today, but that is incidental to what it pins — simplify its fixture and the coverage vanishes silently. Name kept as-is so it still pairs with test_set_frontmatter_field_preserves_triple_dash_in_value, which stays in test_resolve.py.

Adjustments to the moved test, per the destination file's conventions: tmp_path + write_text → the _spec byte-writing helper, verify.* → frontmatter.* (the re-export identity is already pinned by test_verify_re_exports_the_same_exception_object), and read_text → read_bytes().decode() so universal-newline translation can't mask a line-ending change.

Its final assertion also stopped being assert "body text" in ... (d2ca45f, on CodeRabbit's review) — the one substring check in a file whose convention is byte-exact comparison. That is not a style nit: ablating _verified to return yaml.safe_dump(parsed), the round-trip the line edit exists to refuse, reorders the keys and strips the scalar's quotes while leaving body text in place. The substring form passes that corruption — run against the ablated writer to confirm rather than infer it — and the byte-exact form fails it.

Cross-references repaired

• test_resolve.py module docstring: "spec status writer" → "spec field writer".
• The set_frontmatter_status section header becomes a set_frontmatter_field header that says where the status tests went and why these stayed.
• Two set_frontmatter_field docstrings/comments that contrast against set_frontmatter_status now name tests/test_frontmatter.py instead of pointing at a neighbour that left.
• test_frontmatter.py's module docstring loses the paragraph that said this consolidation "is filed as frontmatter: set_frontmatter_status does not preserve the bytes its contract promises #357".
• Not in the original scope, found while checking: tests/test_stories.py:58 pointed at tests/test_resolve.py:346. That reference was correct when written (feat(stories): spec-to-loop engine, adapter, CLI + HITL checkpoints (Phase 2 + 3) #77) and had already rotted to an unrelated assertion through later growth; this PR shifts it again. Replaced with a section name rather than a fresh line number, so it can't rot the same way.

Verification

• uv run pytest tests/test_frontmatter.py tests/test_resolve.py -q → 79 passed.
• Collected-test count moves exactly as the moves predict, checked per file rather than in aggregate: test_frontmatter.py 36 → 37 (+1 moved in), test_resolve.py 46 → 42 (−4), whole suite 3702 → 3699 (−3 = 2 deleted + 1 folded). A silent selector miss would show up here as a mismatch.
• No test-name collision: test_set_frontmatter_status_preserves_triple_dash_in_value resolves to exactly one definition suite-wide. (A cross-file name-reuse audit turns up 13 pre-existing pairs in other modules; none involve these files.)
• Two ablations on the moved test, so it is neither vacuous in its new home nor weaker than the file around it. (1) Replacing _split_frontmatter's standalone-delimiter scan with the naive text.split("---", 2) it exists to refuse makes it FAIL — the property it names is live. (2) The yaml.safe_dump round-trip described above, which the pre-d2ca45f assertion form passed. src/ was restored and confirmed byte-identical after each.
• Full suite uv run pytest -q -n auto: 3675 passed, 24 skipped — run 4× green here, and main run 3× green for comparison. One earlier run failed test_stories_e2e.py::test_e2e_sprint_mode_regression, which did not reproduce in 4 subsequent full runs or in 3 isolated -n auto runs of that file; it is a real-tmux E2E flake with no path to a docstring-and-deletion diff in three unrelated test modules.
• trunk fmt + full trunk check --no-fix: no issues.

Closes #357

[coderabbitai]

Walkthrough

Changes

Frontmatter test coverage

Layer / File(s)	Summary
Status writer characterization tests tests/test_frontmatter.py, CHANGELOG.md	Status rewriting coverage now uses an expanded byte-exact fixture, verifies quoted --- values, and documents the test relocation.
Field writer test organization tests/test_resolve.py, tests/test_stories.py	Field-writer insertion, replacement, comment preservation, and failure behavior remain covered in the resolve tests, with references and descriptions updated.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related issues

• frontmatter: set_frontmatter_status does not preserve the bytes its contract promises #357 — Covers the same set_frontmatter_status byte-preservation tests and their relocation.

Possibly related PRs

• bmad-code-org/bmad-loop#253 — Directly extends tests for frontmatter.set_frontmatter_status, including triple-dash scalar handling.

Suggested reviewers: dracic

Poem

A rabbit moved tests with a hop,
Kept triple-dash titles from a swap.
Frozen blocks stay byte-for-byte,
Field edits keep comments tight.
“No behavior changed!” we cheer—
Fresh frontmatter tests are here!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: consolidating set_frontmatter_status tests into tests/test_frontmatter.py.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch test/consolidate-frontmatter-status-tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[greptile-apps]

Greptile Summary

This is a tests-only consolidation (part 3 of #357): set_frontmatter_status's tests are moved from tests/test_resolve.py to their natural home in tests/test_frontmatter.py, next to the module they exercise. No src/ files change.

• Test disposition: two tests deleted (subsumed by stronger existing assertions including st_mtime_ns and byte-exact checks), one test folded (the richer <frozen-after-approval> fixture absorbed into _PLAIN), one test moved verbatim with mechanical adaptations (bytes write helper, frontmatter.* namespace, full byte comparison replacing substring check).
• Cross-references repaired: test_resolve.py section header and comments updated to point to test_frontmatter.py; test_stories.py comment switched from a now-rotted line number to a stable section name.

Confidence Score: 5/5

Safe to merge — exclusively test and changelog changes with no src/ modifications.

No production code is touched. Every deletion is demonstrably subsumed by a stronger existing assertion in the destination file, the moved test gains a stricter oracle, and the fixture enrichment is consistent with all callers.

Files Needing Attention: No files require special attention.

Important Files Changed

Filename	Overview
tests/test_frontmatter.py	Receives one moved test and a richer _PLAIN fixture; moved test is strictly stronger than its origin (byte-exact comparison vs. substring check).
tests/test_resolve.py	Drops four set_frontmatter_status tests (2 deleted as subsumed, 1 folded, 1 moved); section header and inline comments updated to reflect coverage now lives in test_frontmatter.py.
tests/test_stories.py	Single-line comment fix: stale line-number cross-reference replaced with a stable section-name reference.
CHANGELOG.md	Adds an entry describing the test consolidation as a tests-only, no-behavior-change move.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    subgraph before["Before (test_resolve.py)"]
        R1[test_set_frontmatter_status_replaces]
        R2[test_set_frontmatter_status_idempotent]
        R3[test_set_frontmatter_status_no_frontmatter]
        R4[test_set_frontmatter_status_preserves_triple_dash_in_value]
    end

    subgraph after["After (test_frontmatter.py)"]
        F1[test_a_plain_flip_changes_the_status_line_and_nothing_else]
        F2[test_flipping_to_the_status_already_there_returns_false_and_does_not_write]
        F3[test_no_frontmatter_block_returns_false_with_the_bytes_unchanged]
        F4[test_set_frontmatter_status_preserves_triple_dash_in_value]
    end

    R1 -->|folded| F1
    R2 -->|deleted - subsumed by mtime check| F2
    R3 -->|deleted - subsumed by byte assertion| F3
    R4 -->|moved verbatim| F4

Loading

_{Reviews (2): Last reviewed commit: "test(frontmatter): make the triple-dash ..." | Re-trigger Greptile}

[pbean]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/test_frontmatter.py`:
- Around line 120-129: Update
test_set_frontmatter_status_preserves_triple_dash_in_value to assert the
complete decoded file output equals text.replace("status: in-review", "status:
done") instead of checking only that "body text" is present, while keeping the
existing frontmatter status and title assertions.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: b42ed327-2382-43f1-aac0-64dd6de6a3ec

📥 Commits

Reviewing files that changed from the base of the PR and between 5c4ec15 and af19914.

📒 Files selected for processing (4)

• CHANGELOG.md
• tests/test_frontmatter.py
• tests/test_resolve.py
• tests/test_stories.py

[pbean]

Taken — the substring assert was the weak one, and the ablation makes the case concretely.

Ablating _verified to return yaml.safe_dump(parsed) (the round-trip the line edit exists to refuse) produces:

 ---
-title: 'restore --- review'
-status: in-review
+status: done
+title: restore --- review
 ---
 body text

Keys reordered, the scalar's quotes stripped — and body text still present. The pre-fix form (fm["status"], fm["title"], "body text" in ...) passes that corruption; I ran it against the ablated writer to confirm rather than infer it. The byte-exact comparison fails it. src/ was restored byte-identical afterwards.

Fixed in d2ca45f.