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
19 changes: 19 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,25 @@ story <id>`, the same annotation a sweep bundle writes. Both sprint and stories

### Fixed

- **A deferred finding appended after the last ledger entry is no longer lost (#304).** Found and
first fixed by [@Haven2026](https://github.com/Haven2026) in #274. The inner dev session appends
each review defer as a flat `- source_spec:` / `summary:` / `evidence:` block; landing after the
last canonical `### DW-<n>` entry, it was absorbed into that entry's span, and `parse_legacy()`
masks canonical spans before scanning — so the block was invisible to the sweep's migration
trigger and leftovers read, to `bmad-loop sweep --dry-run`, and to the TUI's legacy view. The session
did its job and recorded the defer; nothing downstream could ever see it. Canonical spans now end
at a flat block.

The boundary recognizes the flat shape exactly as the legacy parser does — the opening line
alone, either bullet marker, whatever `summary:`/`evidence:` lines follow — because a stricter
boundary silently leaves the bug in place for the partial blocks that parser accepts. It is
searched from the entry's own `status:` line and never above it: truncating over the status would
leave the entry reading as neither open nor done, trading a lost flat block for a lost tracked
entry.

`append_entry` also now writes the `location:` field (defaulting to `n/a`, as the orchestrator's
own refiles take it), which the canonical format documents and the writer had been omitting.

- **`return_attached_client` no longer claims a return that never happened (#227).** It discarded
`switch_client`'s result and answered `True` unconditionally, so a failed switch plus a failed
`-l` fallback still journaled the return and sent the sweep unattended while the human sat in
Expand Down
36 changes: 33 additions & 3 deletions src/bmad_loop/deferredwork.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,16 @@

HEADING_RE = re.compile(r"^### (DW-\d+): (.+?)\s*$", re.MULTILINE)
ANY_HEADING_RE = re.compile(r"^#{1,6} ", re.MULTILINE)
# The flat appender's opening line, in the two forms this module needs it: as a
# bullet in the raw ledger (FLAT_ENTRY_RE, the canonical-span boundary in
# parse_ledger) and as bullet *content* after `_BULLET_RE` has stripped the
# marker (`_FLAT_SOURCE_RE`, legacy section). One shape, two anchors — they have
# to agree, or a block the legacy parser recognizes stays invisible to it (#304).
# Keyed on the opening line alone, deliberately: also requiring the block's
# `summary:`/`evidence:` lines would narrow the boundary below the parser's own
# recognition, leaving the bug in place for every partial shape it accepts.
_FLAT_SOURCE_BODY = r"source_spec:[ \t]"
FLAT_ENTRY_RE = re.compile(rf"^[-*][ \t]+{_FLAT_SOURCE_BODY}", re.IGNORECASE | re.MULTILINE)
STATUS_RE = re.compile(r"^status:[ \t]*(.*)$", re.MULTILINE)


Expand Down Expand Up @@ -49,6 +59,18 @@ def parse_ledger(text: str) -> list[DWEntry]:
other = ANY_HEADING_RE.search(text, m.end(), end)
if other:
end = other.start()
# ...and at a flat appender block, which belongs to no canonical entry
# (#304). This span is what parse_legacy() masks out before scanning, so
# absorbing the block hides the finding from every reader of the ledger.
# Searched from the entry's own `status:` line, never from above it:
# truncating over the status leaves the entry reading as neither open nor
# done (open_ids() drops it, classify() calls it malformed), which trades
# one lost flat block for one lost tracked entry. An entry with no status
# line has nothing to protect, so the whole span is fair game.
status_m = STATUS_RE.search(text, m.end(), end)
flat = FLAT_ENTRY_RE.search(text, status_m.end() if status_m else m.end(), end)
if flat:
end = flat.start()
body = text[m.start() : end]
status_m = STATUS_RE.search(body)
entries.append(
Expand Down Expand Up @@ -276,6 +298,7 @@ def append_entry(
origin: str,
source_spec: str,
reason: str,
location: str = "n/a",
status: str = "open",
severity: str | None = None,
) -> str | None:
Expand All @@ -295,7 +318,12 @@ def append_entry(
):
return None
dw_id = f"DW-{next_seq(text)}"
lines = [f"### {dw_id}: {title}", f"origin: {origin}", f"source_spec: `{source_spec}`"]
lines = [
f"### {dw_id}: {title}",
f"origin: {origin}",
f"location: {location}",
f"source_spec: `{source_spec}`",
]
if severity:
lines.append(f"severity: {severity}")
lines.append(f"reason: {reason}")
Expand Down Expand Up @@ -386,8 +414,10 @@ class LegacyEntry:
# summary: <one sentence>
# evidence: <why this is real>
# We recognize it so the `summary` becomes the title (not the source_spec path)
# and the entry migrates cleanly into the canonical `### DW-<seq>` shape.
_FLAT_SOURCE_RE = re.compile(r"^source_spec:[ \t]", re.IGNORECASE)
# and the entry migrates cleanly into the canonical `### DW-<seq>` shape. The
# opening line comes from the same `_FLAT_SOURCE_BODY` as FLAT_ENTRY_RE, which
# bounds canonical spans on it — see that constant for why they must not drift.
_FLAT_SOURCE_RE = re.compile(rf"^{_FLAT_SOURCE_BODY}", re.IGNORECASE)
_FLAT_SUMMARY_RE = re.compile(r"^[ \t]*summary:[ \t]*(.*)$", re.IGNORECASE | re.MULTILINE)
_BULLET_RE = re.compile(r"^[-*][ \t]+(.*)$")
_ITEM_ID_RE = re.compile(
Expand Down
192 changes: 192 additions & 0 deletions tests/test_deferredwork.py
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,178 @@ def test_mixed_ledger_keeps_both_views_separate():
assert all("DW-" not in e.body for e in entries)


def test_flat_append_after_canonical_stays_visible_to_legacy_parser():
text = (
"# Deferred Work\n\n"
"### DW-1: canonical\n\n"
"origin: test\nlocation: n/a\nreason: test\nstatus: open\n\n"
"- source_spec: `spec-next.md`\n"
" summary: later flat finding\n"
" evidence: must not be swallowed by DW-1\n"
)

(canonical,) = parse_ledger(text)
(legacy,) = parse_legacy(text)

assert "later flat finding" not in canonical.body
assert legacy.title == "later flat finding"


# The canonical-span boundary has to recognize every flat shape parse_legacy
# does (#304): a boundary keyed on the full three-line block leaves the bug in
# place for the partial ones — and those are not hypothetical, they are the
# shapes test_flat_appender_missing_summary_falls_back and
# test_flat_appender_in_done_section_is_done already pin. Each case asserts both
# halves of the fix: the block reaches parse_legacy AND DW-1 keeps its status.
FLAT_TAIL_SHAPES = {
"full-block": "- source_spec: `s.md`\n summary: finding\n evidence: e\n",
"no-summary": "- source_spec: `s.md`\n evidence: orphaned note\n",
"no-evidence": "- source_spec: `s.md`\n summary: finding\n",
"swapped-order": "- source_spec: `s.md`\n evidence: e\n summary: finding\n",
"field-interleaved": "- source_spec: `s.md`\n severity: high\n summary: finding\n",
"star-bullet": "* source_spec: `s.md`\n summary: finding\n evidence: e\n",
"tab-after-marker": "-\tsource_spec: `s.md`\n summary: finding\n evidence: e\n",
"padded-marker": "- source_spec: `s.md`\n summary: finding\n evidence: e\n",
"uppercase-field": "- SOURCE_SPEC: `s.md`\n summary: finding\n evidence: e\n",
}


@pytest.mark.parametrize("shape", sorted(FLAT_TAIL_SHAPES), ids=sorted(FLAT_TAIL_SHAPES))
def test_flat_append_visible_for_every_shape_parse_legacy_accepts(shape):
text = (
"# Deferred Work\n\n### DW-1: canonical\n\n"
"origin: test\nlocation: n/a\nreason: test\nstatus: open\n\n" + FLAT_TAIL_SHAPES[shape]
)

(canonical,) = parse_ledger(text)
(legacy,) = parse_legacy(text)

assert canonical.open, "the boundary must not cost DW-1 its status"
assert "source_spec" not in canonical.body
assert legacy.body.lstrip().startswith(("- ", "* ", "-\t"))


def test_flat_boundary_never_truncates_above_the_entry_status_line():
"""A flat-shaped bullet *inside* an entry, ahead of its `status:`, must not
move the span end above that status: an entry parsing as neither open nor
done is a lost tracked entry, the same failure class in the other direction
(open_ids() drops it, classify() reports it malformed)."""
text = (
"### DW-1: canonical\n\norigin: test\nlocation: n/a\n"
"reason: the appender writes this shape, quoted here as evidence\n"
"- source_spec: `quoted-example.md`\n"
" summary: quoted, not a real finding\n"
" evidence: prose inside DW-1, not an appended block\n"
"status: open\n"
)

(canonical,) = parse_ledger(text)

assert canonical.status == "open" and canonical.open
assert "quoted-example.md" in canonical.body
assert open_ids(text) == {"DW-1"}
assert parse_legacy(text) == []


def test_flat_boundary_still_applies_after_a_quoted_block_inside_the_entry():
"""The in-entry bullet does not grant immunity to what follows it: a real
appended block after the entry's `status:` is still bounded out."""
text = (
"### DW-1: canonical\n\norigin: test\nreason: prose\n"
"- source_spec: `quoted-example.md`\n summary: quoted\n evidence: e\n"
"status: open\n\n"
"- source_spec: `real.md`\n summary: real finding\n evidence: e\n"
)

(canonical,) = parse_ledger(text)
(legacy,) = parse_legacy(text)

assert canonical.open
assert "quoted-example.md" in canonical.body and "real finding" not in canonical.body
assert legacy.title == "real finding"


def test_flat_boundary_exposes_the_block_when_the_entry_has_no_status_line():
"""No status line means nothing to protect — the entry is already malformed,
so bounding the block out of it costs nothing and rescues the finding."""
text = "# Deferred Work\n\n### DW-1: malformed\n\norigin: test\nreason: test\n\n" + (
"- source_spec: `s.md`\n summary: rescued finding\n evidence: e\n"
)

(canonical,) = parse_ledger(text)
(legacy,) = parse_legacy(text)

assert canonical.status == "" and not canonical.open
assert legacy.title == "rescued finding"


def test_flat_append_boundary_accepts_crlf():
text = "\r\n".join(
[
"# Deferred Work",
"",
"### DW-1: canonical",
"",
"origin: test",
"location: n/a",
"reason: test",
"status: open",
"",
"- source_spec: `spec-next.md`",
" summary: later flat finding",
" evidence: must not be swallowed by DW-1",
"",
]
)

(canonical,) = parse_ledger(text)
(legacy,) = parse_legacy(text)

assert canonical.open
assert "later flat finding" not in canonical.body
assert legacy.title == "later flat finding"


@pytest.mark.parametrize("gap", ["\n", ""], ids=["blank-line", "no-blank-line"])
def test_flat_append_between_two_canonical_entries(gap):
"""The realistic sequence: the inner session defers (flat) and the
orchestrator then refiles a canonical entry at EOF, sandwiching the block."""
text = (
"# Deferred Work\n\n### DW-1: a\n\norigin: t\nreason: t\nstatus: open\n"
+ gap
+ "- source_spec: `s.md`\n summary: sandwiched finding\n evidence: e\n"
+ "\n### DW-2: b\n\norigin: t\nreason: t\nstatus: open\n"
)

assert open_ids(text) == {"DW-1", "DW-2"}
(legacy,) = parse_legacy(text)
assert legacy.title == "sandwiched finding"


def test_two_flat_blocks_after_the_last_entry_are_both_visible():
block = "- source_spec: `s.md`\n summary: %s\n evidence: e\n"
text = (
"# Deferred Work\n\n### DW-1: a\n\norigin: t\nreason: t\nstatus: open\n\n"
+ (block % "first")
+ (block % "second")
)

assert open_ids(text) == {"DW-1"}
assert [e.title for e in parse_legacy(text)] == ["first", "second"]


def test_flat_append_followed_by_a_legacy_section_keeps_both():
text = (
"# Deferred Work\n\n### DW-1: a\n\norigin: t\nreason: t\nstatus: open\n\n"
"- source_spec: `s.md`\n summary: flat finding\n evidence: e\n"
"\n## Deferred from: an older review (2026-06-01)\n\n"
"- W1 — a freeform legacy item\n"
)

assert open_ids(text) == {"DW-1"}
assert [e.title for e in parse_legacy(text)] == ["flat finding", "a freeform legacy item"]


def test_legacy_item_does_not_swallow_masked_canonical_neighbor():
text = (
"## Deferred from: somewhere (2026-06-01)\n\n"
Expand Down Expand Up @@ -419,11 +591,31 @@ def test_append_entry_numbers_and_writes(tmp_path):
assert "DW-5" in entries and entries["DW-5"].open
body = entries["DW-5"].body
assert "origin: review-budget-followup" in body
assert "location: n/a" in body
assert "source_spec: `spec-foo.md`" in body
assert "severity: low" in body
assert "follow-up still recommended for dw-x" in body


def test_append_entry_preserves_a_supplied_location(tmp_path):
"""`n/a` is only the default the orchestrator's own refiles take — the field
is documented as `<file:line or component>` and must round-trip verbatim."""
p = tmp_path / "deferred-work.md"
assert (
append_entry(
p,
title="follow-up",
origin="code-review",
source_spec="spec-foo.md",
reason="still open",
location="src/foo.py:12",
)
== "DW-1"
)
(entry,) = parse_ledger(p.read_text())
assert "location: src/foo.py:12" in entry.body


def test_append_entry_idempotent_for_open_origin_and_spec(tmp_path):
p = tmp_path / "deferred-work.md"
p.write_text("# Deferred Work\n")
Expand Down