generated from coulomb/repo-seed
fix(consistency): fix C-04 status vocabulary mismatch + surface PATCH errors
Root cause: workplan files use "done" (task vocabulary) but the DB workstream
API only accepts "completed". The PATCH was silently failing with 422.
Fixes:
- Add FILE_TO_DB_WORKSTREAM_STATUS map and normalise_workstream_status()
- Normalise file status before C-04 comparison: done↔completed is no longer
spurious drift
- Normalise file status before PATCHing: always send DB-valid "completed"
- _api_patch now returns {"_error": ...} instead of None on failure, so the
fix loop reports FAILED entries rather than silently dropping them
- 9 new tests in TestNormaliseWorkstreamStatus (42 total)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -59,6 +59,18 @@ VALID_WP_STATUSES = {"active", "completed", "archived"}
|
||||
VALID_TASK_STATUSES = {"todo", "in_progress", "blocked", "done", "cancelled"}
|
||||
VALID_TASK_PRIORITIES = {"low", "medium", "high", "critical"}
|
||||
|
||||
# Workplan files use task-style vocabulary ("done"); the DB workstream API uses
|
||||
# "completed". This map translates file values to DB values before comparison
|
||||
# and before PATCHing, so "done" vs "completed" is never flagged as C-04 drift.
|
||||
FILE_TO_DB_WORKSTREAM_STATUS: dict[str, str] = {
|
||||
"done": "completed",
|
||||
}
|
||||
|
||||
|
||||
def normalise_workstream_status(status: str) -> str:
|
||||
"""Translate a workplan file status value to its DB-canonical equivalent."""
|
||||
return FILE_TO_DB_WORKSTREAM_STATUS.get(status, status)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Data types
|
||||
@@ -248,8 +260,10 @@ def _api_patch(api_base: str, path: str, body: dict) -> Any:
|
||||
r = c.patch(path, json=body)
|
||||
r.raise_for_status()
|
||||
return r.json()
|
||||
except Exception:
|
||||
return None
|
||||
except Exception as exc:
|
||||
# Return a sentinel dict so callers can distinguish "API error" from "success"
|
||||
# and report it rather than silently dropping the fix.
|
||||
return {"_error": str(exc)}
|
||||
|
||||
|
||||
def _api_post(api_base: str, path: str, body: dict) -> Any:
|
||||
@@ -393,9 +407,11 @@ def check_repo(api_base: str, repo_slug: str) -> ConsistencyReport:
|
||||
)
|
||||
# Continue to check drift even with mismatched repo
|
||||
|
||||
# C-04: status drift
|
||||
# C-04: status drift — normalise file value before comparing so that
|
||||
# "done" (file) vs "completed" (DB) is not treated as drift.
|
||||
db_status = ws.get("status", "")
|
||||
if file_status and db_status and file_status != db_status:
|
||||
normalised_file_status = normalise_workstream_status(file_status)
|
||||
if file_status and db_status and normalised_file_status != db_status:
|
||||
report.add(
|
||||
severity="WARN", check_id="C-04",
|
||||
message=(
|
||||
@@ -407,7 +423,11 @@ def check_repo(api_base: str, repo_slug: str) -> ConsistencyReport:
|
||||
file_value=file_status,
|
||||
db_value=db_status,
|
||||
fixable=True,
|
||||
_fix_context={"ws_id": ws_id, "field": "status", "value": file_status},
|
||||
_fix_context={
|
||||
"ws_id": ws_id,
|
||||
"field": "status",
|
||||
"value": normalised_file_status, # always send DB-valid value
|
||||
},
|
||||
)
|
||||
|
||||
# C-05: title drift
|
||||
@@ -560,11 +580,16 @@ def fix_repo(api_base: str, repo_slug: str) -> ConsistencyReport:
|
||||
ws_id = ctx["ws_id"]
|
||||
result = _api_patch(api_base, f"/workstreams/{ws_id}",
|
||||
{ctx["field"]: ctx["value"]})
|
||||
if result is not None:
|
||||
if result is not None and "_error" not in result:
|
||||
report.fixes_applied.append(
|
||||
f"{issue.check_id} fixed: workstream {ws_id[:8]}… "
|
||||
f"{ctx['field']} → {ctx['value']!r}"
|
||||
)
|
||||
elif result is not None:
|
||||
report.fixes_applied.append(
|
||||
f"{issue.check_id} FAILED: workstream {ws_id[:8]}… "
|
||||
f"{ctx['field']} → {ctx['value']!r}: {result['_error']}"
|
||||
)
|
||||
|
||||
elif issue.check_id == "C-06":
|
||||
wp_file = Path(ctx["wp_file"])
|
||||
|
||||
@@ -23,7 +23,9 @@ sys.path.insert(0, str(Path(__file__).parent.parent / "scripts"))
|
||||
from consistency_check import (
|
||||
ConsistencyReport,
|
||||
Issue,
|
||||
FILE_TO_DB_WORKSTREAM_STATUS,
|
||||
get_tasks_from_workplan,
|
||||
normalise_workstream_status,
|
||||
parse_frontmatter,
|
||||
parse_task_blocks,
|
||||
render_text,
|
||||
@@ -326,3 +328,46 @@ class TestReportToDict:
|
||||
d = report_to_dict(r)
|
||||
assert d["repo_slug"] == "the-custodian"
|
||||
assert d["repo_path"] == "/home/worsch/the-custodian"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Status vocabulary normalisation
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
class TestNormaliseWorkstreamStatus:
|
||||
"""FILE_TO_DB_WORKSTREAM_STATUS maps workplan file vocabulary to DB vocabulary.
|
||||
|
||||
Workplan files use task-style "done"; the DB workstream API uses "completed".
|
||||
The C-04 check and fix code must normalise before comparing or PATCHing.
|
||||
"""
|
||||
|
||||
def test_done_maps_to_completed(self):
|
||||
assert normalise_workstream_status("done") == "completed"
|
||||
|
||||
def test_completed_is_identity(self):
|
||||
assert normalise_workstream_status("completed") == "completed"
|
||||
|
||||
def test_active_is_identity(self):
|
||||
assert normalise_workstream_status("active") == "active"
|
||||
|
||||
def test_blocked_is_identity(self):
|
||||
assert normalise_workstream_status("blocked") == "blocked"
|
||||
|
||||
def test_archived_is_identity(self):
|
||||
assert normalise_workstream_status("archived") == "archived"
|
||||
|
||||
def test_unknown_value_returned_as_is(self):
|
||||
# Don't crash on unexpected values — return them unchanged
|
||||
assert normalise_workstream_status("foobar") == "foobar"
|
||||
|
||||
def test_map_constant_covers_done(self):
|
||||
assert "done" in FILE_TO_DB_WORKSTREAM_STATUS
|
||||
assert FILE_TO_DB_WORKSTREAM_STATUS["done"] == "completed"
|
||||
|
||||
def test_c04_no_spurious_drift_when_done_vs_completed(self):
|
||||
"""done (file) vs completed (DB) must NOT be reported as C-04 drift."""
|
||||
assert normalise_workstream_status("done") == normalise_workstream_status("completed")
|
||||
|
||||
def test_c04_real_drift_still_detected(self):
|
||||
"""done (file) vs active (DB) IS real drift and must be detected."""
|
||||
assert normalise_workstream_status("done") != normalise_workstream_status("active")
|
||||
|
||||
Reference in New Issue
Block a user