fix(consistency): preserve repository domain in briefs
Assistant: codex Assistant-Model: gpt-5.6-sol Assistant-Session: 01a02b22-9638-76d2-bbff-b7ea1770b118
This commit is contained in:
parent
b91b8705d1
commit
fae2816199
3 changed files with 72 additions and 4 deletions
|
|
@ -2547,10 +2547,10 @@ def _write_custodian_brief(api_base: str, repo_slug: str, repo_path: str) -> boo
|
||||||
return False
|
return False
|
||||||
|
|
||||||
repo_id: str = repo.get("id", "")
|
repo_id: str = repo.get("id", "")
|
||||||
domain_slug: str = ""
|
domain_slug = str(repo.get("domain_slug") or "")
|
||||||
|
|
||||||
# Resolve domain slug: prefer active workstreams, fall back to any workstream
|
# The repository projection owns domain identity. Retain the workplan/topic
|
||||||
# so that a fully-finished repo doesn't degrade to "(unknown)".
|
# lookup only for compatibility with older API responses that omitted it.
|
||||||
workstreams: list[dict] = []
|
workstreams: list[dict] = []
|
||||||
for status in OPEN_WORKSTREAM_STATUSES:
|
for status in OPEN_WORKSTREAM_STATUSES:
|
||||||
rows = _api_get(api_base, "/workplans", {"repo_id": repo_id, "status": status}) or []
|
rows = _api_get(api_base, "/workplans", {"repo_id": repo_id, "status": status}) or []
|
||||||
|
|
@ -2560,7 +2560,7 @@ def _write_custodian_brief(api_base: str, repo_slug: str, repo_path: str) -> boo
|
||||||
if not _ws_for_domain:
|
if not _ws_for_domain:
|
||||||
all_ws = _api_get(api_base, "/workplans", {"repo_id": repo_id}) or []
|
all_ws = _api_get(api_base, "/workplans", {"repo_id": repo_id}) or []
|
||||||
_ws_for_domain = all_ws if isinstance(all_ws, list) else []
|
_ws_for_domain = all_ws if isinstance(all_ws, list) else []
|
||||||
if _ws_for_domain:
|
if not domain_slug and _ws_for_domain:
|
||||||
topic = _api_get(api_base, f"/topics/{_ws_for_domain[0].get('topic_id', '')}")
|
topic = _api_get(api_base, f"/topics/{_ws_for_domain[0].get('topic_id', '')}")
|
||||||
if topic:
|
if topic:
|
||||||
domain_slug = topic.get("domain_slug", "")
|
domain_slug = topic.get("domain_slug", "")
|
||||||
|
|
|
||||||
|
|
@ -41,6 +41,7 @@ from consistency_check import (
|
||||||
_patch_frontmatter_field,
|
_patch_frontmatter_field,
|
||||||
_patch_task_status_in_file,
|
_patch_task_status_in_file,
|
||||||
_report_needs_action,
|
_report_needs_action,
|
||||||
|
_write_custodian_brief,
|
||||||
archive_closed_workplans,
|
archive_closed_workplans,
|
||||||
canonical_workplan_filename,
|
canonical_workplan_filename,
|
||||||
check_repo,
|
check_repo,
|
||||||
|
|
@ -72,6 +73,63 @@ class TestResolveTopicDomainSlug:
|
||||||
assert resolve_topic_domain_slug("custodian", repo_market_domain="infotech") == "infotech"
|
assert resolve_topic_domain_slug("custodian", repo_market_domain="infotech") == "infotech"
|
||||||
|
|
||||||
|
|
||||||
|
class TestCustodianBriefDomain:
|
||||||
|
@staticmethod
|
||||||
|
def _install_common_mocks(monkeypatch, calls, repo_payload, topic_payload=None):
|
||||||
|
def fake_get(_api_base, path, params=None):
|
||||||
|
calls.append((path, params))
|
||||||
|
if path == "/repos/demo":
|
||||||
|
return repo_payload
|
||||||
|
if path == "/workplans":
|
||||||
|
return [{"id": "wp-1", "title": "Open", "topic_id": None}]
|
||||||
|
if path == "/tasks":
|
||||||
|
return []
|
||||||
|
if path == "/repo-goals":
|
||||||
|
return []
|
||||||
|
if path.startswith("/topics/"):
|
||||||
|
return topic_payload
|
||||||
|
if path == "/messages":
|
||||||
|
return []
|
||||||
|
return None
|
||||||
|
|
||||||
|
monkeypatch.setattr("consistency_check._api_get", fake_get)
|
||||||
|
monkeypatch.setattr(
|
||||||
|
"consistency_check._git_commit_writeback", lambda *args, **kwargs: True
|
||||||
|
)
|
||||||
|
|
||||||
|
def test_brief_prefers_repository_domain_when_workplan_topic_is_null(
|
||||||
|
self, tmp_path, monkeypatch
|
||||||
|
):
|
||||||
|
calls = []
|
||||||
|
self._install_common_mocks(
|
||||||
|
monkeypatch,
|
||||||
|
calls,
|
||||||
|
{"id": "repo-1", "domain_slug": "infotech"},
|
||||||
|
)
|
||||||
|
|
||||||
|
assert _write_custodian_brief("http://unused", "demo", str(tmp_path)) is True
|
||||||
|
brief = (tmp_path / ".custodian-brief.md").read_text(encoding="utf-8")
|
||||||
|
assert "**Domain:** infotech" in brief
|
||||||
|
assert '`get_domain_summary("infotech")`' in brief
|
||||||
|
assert not any(path.startswith("/topics/") for path, _params in calls)
|
||||||
|
|
||||||
|
def test_brief_retains_topic_fallback_for_legacy_repo_response(
|
||||||
|
self, tmp_path, monkeypatch
|
||||||
|
):
|
||||||
|
calls = []
|
||||||
|
self._install_common_mocks(
|
||||||
|
monkeypatch,
|
||||||
|
calls,
|
||||||
|
{"id": "repo-1"},
|
||||||
|
{"domain_slug": "financials"},
|
||||||
|
)
|
||||||
|
|
||||||
|
assert _write_custodian_brief("http://unused", "demo", str(tmp_path)) is True
|
||||||
|
brief = (tmp_path / ".custodian-brief.md").read_text(encoding="utf-8")
|
||||||
|
assert "**Domain:** financials" in brief
|
||||||
|
assert any(path.startswith("/topics/") for path, _params in calls)
|
||||||
|
|
||||||
|
|
||||||
class TestRepoManagerConformanceAdapter:
|
class TestRepoManagerConformanceAdapter:
|
||||||
def test_surfaces_repo_manager_findings_without_reimplementing_rules(self, tmp_path, monkeypatch):
|
def test_surfaces_repo_manager_findings_without_reimplementing_rules(self, tmp_path, monkeypatch):
|
||||||
rmgr = tmp_path / "rmgr"
|
rmgr = tmp_path / "rmgr"
|
||||||
|
|
|
||||||
|
|
@ -346,6 +346,16 @@ project repos are already routed to `rmgr scaffold`. Gate the deletion on
|
||||||
`RMGR-WP-0004-T06` proving byte-identical durable output and idempotent
|
`RMGR-WP-0004-T06` proving byte-identical durable output and idempotent
|
||||||
re-scaffolding.
|
re-scaffolding.
|
||||||
|
|
||||||
|
**Brief compatibility repair (2026-08-23):** the surviving consistency
|
||||||
|
generator degraded `the-custodian` to `Domain: (unknown)` and emitted
|
||||||
|
`get_domain_summary("")` even though `GET /repos/the-custodian` correctly
|
||||||
|
projected `domain_slug: infotech`. Active workplans can legitimately have no
|
||||||
|
topic, so topic inference is not authoritative. The generator now prefers the
|
||||||
|
repository projection and retains topic lookup only for legacy API responses
|
||||||
|
that omit `domain_slug`. Two regression tests cover both paths; all 131
|
||||||
|
consistency-check tests pass. This is a compatibility fix until the generator
|
||||||
|
moves or retires, not new permanent State Hub authority.
|
||||||
|
|
||||||
## Retire legacy surfaces
|
## Retire legacy surfaces
|
||||||
|
|
||||||
```task
|
```task
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue