Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,10 @@ story <id>`, the same annotation a sweep bundle writes. Both sprint and stories

### Changed

- **`set_frontmatter_status`'s tests now live in `tests/test_frontmatter.py` (#357, part 3).** They
had stayed in `tests/test_resolve.py` next to `set_frontmatter_field`'s so parts 1 and 2 read as
changes rather than as a rename. Tests only — no behavior change.

- **The story token budget is checked while the story runs (#336).** `max_tokens_per_story` was
read once, after the story had already been marked done — so an overrun was reported only after
every token was spent, and a story that deferred or escalated was never checked at all (one field
Expand Down
29 changes: 22 additions & 7 deletions tests/test_frontmatter.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,17 @@
half pins the rewrite: a writer that verifies its own edit by re-parsing, and
RAISES rather than returning a `False` nobody reads when the reader can see a
status it cannot safely move.

`set_frontmatter_status`'s original tests live in `tests/test_resolve.py:63-138`,
next to `set_frontmatter_field`'s. They were deliberately left there —
characterize before restructuring, and moving them would hide this diff behind a
rename. Consolidating them here is filed as #357.
"""

import pytest
import yaml

from bmad_loop import frontmatter, verify

_PLAIN = "---\ntitle: List command\nstatus: in-review\nowner: amelia\n---\n\n# Spec\n\nbody\n"
_PLAIN = (
"---\ntitle: List command\nstatus: in-review\nowner: amelia\n---\n\n# Spec\n\n"
"<frozen-after-approval>\nFilter notes by workspace name.\n</frozen-after-approval>\n"
)


def _spec(tmp_path, text: str, name: str = "spec.md"):
Expand All @@ -43,7 +41,12 @@ def _spec(tmp_path, text: str, name: str = "spec.md"):

def test_a_plain_flip_changes_the_status_line_and_nothing_else(tmp_path):
"""The whole point of a line edit over a YAML round-trip: field order,
comments, quoting and body survive byte-for-byte."""
comments, quoting and body survive byte-for-byte.

The fixture carries the shape the orchestrator actually flips — the other
frontmatter fields plus a `<frozen-after-approval>` body block the writer must
never reach. One byte-exact comparison subsumes any list of per-field
substring assertions: it also fails on what such a list forgot to name."""
spec = _spec(tmp_path, _PLAIN)
assert frontmatter.set_frontmatter_status(spec, "done") is True
assert spec.read_bytes().decode() == _PLAIN.replace("status: in-review", "status: done")
Expand Down Expand Up @@ -114,6 +117,18 @@ def test_indentation_on_the_status_line_survives(tmp_path):
assert spec.read_bytes().decode() == "---\n status: done\n---\nbody\n"


def test_set_frontmatter_status_preserves_triple_dash_in_value(tmp_path):
"""A `---` inside a scalar is not the closing delimiter: status flips and the
---bearing title + body survive (a plain split("---", 2) corrupted this)."""
text = "---\ntitle: 'restore --- review'\nstatus: in-review\n---\nbody text\n"
spec = _spec(tmp_path, text)
assert frontmatter.set_frontmatter_status(spec, "done") is True
fm = frontmatter.read_frontmatter(spec)
assert fm["status"] == "done"
assert fm["title"] == "restore --- review" # scalar with --- intact
assert spec.read_bytes().decode() == text.replace("status: in-review", "status: done")


# =============================================================== line endings
#
# The half of "only the status value changes" the writer used to break on every
Expand Down
60 changes: 13 additions & 47 deletions tests/test_resolve.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
"""Escalation-resolution: context build, re-arm, spec status writer, session."""
"""Escalation-resolution: context build, re-arm, spec field writer, session."""

import json

Expand Down Expand Up @@ -60,41 +60,21 @@ def _escalated_run(
return run.run_dir, run.state, run.task


# ----------------------------------------------------------- set_frontmatter_status


def test_set_frontmatter_status_replaces(tmp_path):
spec = tmp_path / "spec.md"
spec.write_text(SPEC, encoding="utf-8")
assert verify.set_frontmatter_status(spec, "ready-for-dev") is True
assert verify.read_frontmatter(spec)["status"] == "ready-for-dev"
# other fields + the frozen block survive untouched
text = spec.read_text(encoding="utf-8")
assert "owner: amelia" in text
assert "<frozen-after-approval>" in text
assert "title: List command" in text


def test_set_frontmatter_status_idempotent(tmp_path):
spec = tmp_path / "spec.md"
spec.write_text(SPEC, encoding="utf-8")
verify.set_frontmatter_status(spec, "ready-for-dev")
# second call is a no-op (already at the target)
assert verify.set_frontmatter_status(spec, "ready-for-dev") is False


def test_set_frontmatter_status_no_frontmatter(tmp_path):
spec = tmp_path / "spec.md"
spec.write_text("# just a heading\n", encoding="utf-8")
assert verify.set_frontmatter_status(spec, "ready-for-dev") is False
# ------------------------------------------------------------ set_frontmatter_field
#
# `set_frontmatter_status`'s own tests live in tests/test_frontmatter.py, next to
# the module that defines it (#357). What stays here is `set_frontmatter_field`,
# which is `verify`'s — it shares the renderer and the verified-edit core, so the
# tests below are about the half that differs: insert-on-miss.


def test_set_frontmatter_field_replaces_inserts_idempotent(tmp_path):
spec = tmp_path / "spec.md"
spec.write_text(SPEC, encoding="utf-8")
assert verify.set_frontmatter_field(spec, "owner", "winston") is True
assert verify.read_frontmatter(spec)["owner"] == "winston"
# unlike set_frontmatter_status, a missing key is INSERTED (block's last line)
# unlike set_frontmatter_status, a missing key is INSERTED (block's last line);
# its refusal to invent one is pinned in tests/test_frontmatter.py
assert verify.set_frontmatter_field(spec, "baseline_revision", "abc123") is True
fm = verify.read_frontmatter(spec)
assert fm["baseline_revision"] == "abc123"
Expand All @@ -107,21 +87,6 @@ def test_set_frontmatter_field_replaces_inserts_idempotent(tmp_path):
assert verify.set_frontmatter_field(bare, "baseline_revision", "abc123") is False


def test_set_frontmatter_status_preserves_triple_dash_in_value(tmp_path):
"""A `---` inside a scalar is not the closing delimiter: status flips and the
---bearing title + body survive (a plain split("---", 2) corrupted this)."""
spec = tmp_path / "spec.md"
spec.write_text(
"---\ntitle: 'restore --- review'\nstatus: in-review\n---\nbody text\n",
encoding="utf-8",
)
assert verify.set_frontmatter_status(spec, "done") is True
fm = verify.read_frontmatter(spec)
assert fm["status"] == "done"
assert fm["title"] == "restore --- review" # scalar with --- intact
assert "body text" in spec.read_text(encoding="utf-8")


def test_set_frontmatter_field_preserves_a_trailing_inline_comment(tmp_path):
"""This helper's docstring has always promised "comments survive"; until #357
part 2 that was true of every line except the one it edited. It shares
Expand Down Expand Up @@ -149,9 +114,10 @@ def test_set_frontmatter_field_rewrites_a_quoted_key_instead_of_duplicating_it(t


def test_set_frontmatter_field_refuses_a_key_no_line_edit_can_move(tmp_path):
"""Same three-way contract as `set_frontmatter_status`: False means nothing
to change, and a field the reader CAN see in an unrewritable shape raises
instead of silently appending a duplicate the reader would never resolve."""
"""Same three-way contract as `set_frontmatter_status` (pinned in
tests/test_frontmatter.py): False means nothing to change, and a field the
reader CAN see in an unrewritable shape raises instead of silently appending
a duplicate the reader would never resolve."""
spec = tmp_path / "spec.md"
original = "---\n{baseline_revision: old, keep: 1}\n---\nbody\n"
spec.write_text(original, encoding="utf-8")
Expand Down
3 changes: 2 additions & 1 deletion tests/test_stories.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,8 @@ def test_load_missing_file_raises_pinned_message(tmp_path):
# A manifest / spec is agent- or human-authored, so it can hold non-UTF-8 bytes.
# `read_text(encoding="utf-8")` raises UnicodeDecodeError (a ValueError, NOT a
# yaml.YAMLError), so the stories-mode reads must surface it as a clean error / degrade
# rather than crash preflight/dry-run/status. Mirrors tests/test_resolve.py:346.
# rather than crash preflight/dry-run/status. Mirrors the "non-UTF-8 robustness"
# section of tests/test_resolve.py.
_BAD_UTF8 = b"\xff\xfe\x00\x01 not utf-8 \x80\x81"


Expand Down