From 7b7b725f8bf4a9bf2cd7dd798d928dded2e25a03 Mon Sep 17 00:00:00 2001 From: tegwick Date: Thu, 12 Mar 2026 21:57:11 +0100 Subject: [PATCH] fix(consistency): fix C-04 status vocabulary mismatch + surface PATCH errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- scripts/consistency_check.py | 37 ++++++++++++++++++++++----- tests/test_consistency_check.py | 45 +++++++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+), 6 deletions(-) diff --git a/scripts/consistency_check.py b/scripts/consistency_check.py index f30f7da..e160ef6 100644 --- a/scripts/consistency_check.py +++ b/scripts/consistency_check.py @@ -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"]) diff --git a/tests/test_consistency_check.py b/tests/test_consistency_check.py index a03c9b7..f25c62d 100644 --- a/tests/test_consistency_check.py +++ b/tests/test_consistency_check.py @@ -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")