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_STATUSES = {"todo", "in_progress", "blocked", "done", "cancelled"}
|
||||||
VALID_TASK_PRIORITIES = {"low", "medium", "high", "critical"}
|
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
|
# Data types
|
||||||
@@ -248,8 +260,10 @@ def _api_patch(api_base: str, path: str, body: dict) -> Any:
|
|||||||
r = c.patch(path, json=body)
|
r = c.patch(path, json=body)
|
||||||
r.raise_for_status()
|
r.raise_for_status()
|
||||||
return r.json()
|
return r.json()
|
||||||
except Exception:
|
except Exception as exc:
|
||||||
return None
|
# 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:
|
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
|
# 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", "")
|
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(
|
report.add(
|
||||||
severity="WARN", check_id="C-04",
|
severity="WARN", check_id="C-04",
|
||||||
message=(
|
message=(
|
||||||
@@ -407,7 +423,11 @@ def check_repo(api_base: str, repo_slug: str) -> ConsistencyReport:
|
|||||||
file_value=file_status,
|
file_value=file_status,
|
||||||
db_value=db_status,
|
db_value=db_status,
|
||||||
fixable=True,
|
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
|
# C-05: title drift
|
||||||
@@ -560,11 +580,16 @@ def fix_repo(api_base: str, repo_slug: str) -> ConsistencyReport:
|
|||||||
ws_id = ctx["ws_id"]
|
ws_id = ctx["ws_id"]
|
||||||
result = _api_patch(api_base, f"/workstreams/{ws_id}",
|
result = _api_patch(api_base, f"/workstreams/{ws_id}",
|
||||||
{ctx["field"]: ctx["value"]})
|
{ctx["field"]: ctx["value"]})
|
||||||
if result is not None:
|
if result is not None and "_error" not in result:
|
||||||
report.fixes_applied.append(
|
report.fixes_applied.append(
|
||||||
f"{issue.check_id} fixed: workstream {ws_id[:8]}… "
|
f"{issue.check_id} fixed: workstream {ws_id[:8]}… "
|
||||||
f"{ctx['field']} → {ctx['value']!r}"
|
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":
|
elif issue.check_id == "C-06":
|
||||||
wp_file = Path(ctx["wp_file"])
|
wp_file = Path(ctx["wp_file"])
|
||||||
|
|||||||
@@ -23,7 +23,9 @@ sys.path.insert(0, str(Path(__file__).parent.parent / "scripts"))
|
|||||||
from consistency_check import (
|
from consistency_check import (
|
||||||
ConsistencyReport,
|
ConsistencyReport,
|
||||||
Issue,
|
Issue,
|
||||||
|
FILE_TO_DB_WORKSTREAM_STATUS,
|
||||||
get_tasks_from_workplan,
|
get_tasks_from_workplan,
|
||||||
|
normalise_workstream_status,
|
||||||
parse_frontmatter,
|
parse_frontmatter,
|
||||||
parse_task_blocks,
|
parse_task_blocks,
|
||||||
render_text,
|
render_text,
|
||||||
@@ -326,3 +328,46 @@ class TestReportToDict:
|
|||||||
d = report_to_dict(r)
|
d = report_to_dict(r)
|
||||||
assert d["repo_slug"] == "the-custodian"
|
assert d["repo_slug"] == "the-custodian"
|
||||||
assert d["repo_path"] == "/home/worsch/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