feat(tasks): give task rows a canonical record identifier
Every work-record type carried a stable identifier except tasks, whose rows held only id, workplan_id, title, status and priority — nothing connecting a row to CUST-WP-0067-T01 in the file it came from. Matching was therefore by title, so a renamed heading looked like one task vanishing and another appearing, and the forge-derived reset had to refuse to touch tasks at all. Adds tasks.record_id (nullable: no migration can invent an identity for an existing row) and a backfill that reads the pairing from the repository files, where a task declares both its canonical id and its projection UUID. 5516 pairs across 121 repositories with zero conflicts; 4456 of 6073 cache task rows identified. Diff and reset now key on record_id where present, falling back to a title-prefixed key so an unidentified row stays visibly unidentified. Unknown stays unknown: a row the files do not claim keeps no identity and the reset keeps refusing to act on it, and an existing identity is never overwritten — a mismatch is recorded as a conflict rather than resolved. Refs STATE-WP-0083-T06 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Assistant: claude-code Assistant-Model: opus Assistant-Process: 2583210@bnt-lap001 Assistant-Session: f2bff2d5-e9b2-4338-92ca-10282a927006
This commit is contained in:
parent
43ffe883c3
commit
8b207a991a
6 changed files with 325 additions and 6 deletions
|
|
@ -36,6 +36,12 @@ class Task(Base, TimestampMixin):
|
||||||
nullable=False,
|
nullable=False,
|
||||||
index=True,
|
index=True,
|
||||||
)
|
)
|
||||||
|
# Canonical work-record identifier, e.g. "CUST-WP-0067-T01" (ADR-007).
|
||||||
|
# Nullable because rows created before STATE-WP-0083-T06 have none; only the
|
||||||
|
# repository files know the mapping. Without it a task can be matched only by
|
||||||
|
# title, so renaming a heading looks like one task vanishing and another
|
||||||
|
# appearing.
|
||||||
|
record_id: Mapped[str | None] = mapped_column(String(120), nullable=True, index=True)
|
||||||
title: Mapped[str] = mapped_column(String(255), nullable=False)
|
title: Mapped[str] = mapped_column(String(255), nullable=False)
|
||||||
description: Mapped[str | None] = mapped_column(Text, nullable=True)
|
description: Mapped[str | None] = mapped_column(Text, nullable=True)
|
||||||
status: Mapped[TaskStatus] = mapped_column(
|
status: Mapped[TaskStatus] = mapped_column(
|
||||||
|
|
|
||||||
|
|
@ -350,11 +350,19 @@ def diff_against_hub(
|
||||||
cur = have_wp.get(w.record_id.strip().lower())
|
cur = have_wp.get(w.record_id.strip().lower())
|
||||||
if cur:
|
if cur:
|
||||||
hub_rows = hub_tasks_by_workplan.get(str(cur["id"]), [])
|
hub_rows = hub_tasks_by_workplan.get(str(cur["id"]), [])
|
||||||
# Tasks carry no canonical id on the hub, only a title, so compare on
|
# Match on the canonical record id where the hub has one. Rows created
|
||||||
# title. Imperfect, and the reason task removal needs the same explicit
|
# before STATE-WP-0083-T06 fall back to title, which is why those are
|
||||||
# acknowledgement as everything else.
|
# reported rather than acted on: a renamed heading is indistinguishable
|
||||||
have = {(t.get("title") or "").strip().lower(): t for t in hub_rows}
|
# from a replaced task under title matching.
|
||||||
want = {(t.title or t.record_id).strip().lower(): t for t in w.tasks}
|
def _task_key(record_id: str | None, title: str | None) -> str:
|
||||||
|
if record_id:
|
||||||
|
return record_id.strip().lower()
|
||||||
|
return "title:" + (title or "").strip().lower()
|
||||||
|
|
||||||
|
have = {
|
||||||
|
_task_key(t.get("record_id"), t.get("title")): t for t in hub_rows
|
||||||
|
}
|
||||||
|
want = {_task_key(t.record_id, t.title): t for t in w.tasks}
|
||||||
for key, t in want.items():
|
for key, t in want.items():
|
||||||
if key not in have:
|
if key not in have:
|
||||||
d.missing.append(
|
d.missing.append(
|
||||||
|
|
@ -530,6 +538,7 @@ async def reset_repository_projection(
|
||||||
Task(
|
Task(
|
||||||
id=uuid.UUID(t.uuid),
|
id=uuid.UUID(t.uuid),
|
||||||
workplan_id=row.id,
|
workplan_id=row.id,
|
||||||
|
record_id=t.record_id,
|
||||||
title=t.title or t.record_id,
|
title=t.title or t.record_id,
|
||||||
status=t.status or "todo",
|
status=t.status or "todo",
|
||||||
priority=t.priority or "medium",
|
priority=t.priority or "medium",
|
||||||
|
|
|
||||||
116
api/services/task_record_id_backfill.py
Normal file
116
api/services/task_record_id_backfill.py
Normal file
|
|
@ -0,0 +1,116 @@
|
||||||
|
"""Backfill canonical record ids onto existing task rows (STATE-WP-0083-T06).
|
||||||
|
|
||||||
|
Only the repository files hold the mapping. A file task declares both its
|
||||||
|
canonical id and the projection UUID it was registered under:
|
||||||
|
|
||||||
|
```task
|
||||||
|
id: CUST-WP-0067-T01
|
||||||
|
state_hub_task_id: "f3608db4-..."
|
||||||
|
```
|
||||||
|
|
||||||
|
so the pairing can be read directly rather than guessed from titles. Anything a
|
||||||
|
file does not claim is left alone: a task row whose canonical id cannot be
|
||||||
|
established keeps `record_id` null, and the reset continues to refuse to act on
|
||||||
|
it. An unknown identity must stay unknown rather than be inferred.
|
||||||
|
"""
|
||||||
|
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import re
|
||||||
|
from dataclasses import dataclass, field
|
||||||
|
from pathlib import Path
|
||||||
|
from typing import Any
|
||||||
|
|
||||||
|
_TASK_BLOCK_RE = re.compile(r"```task\s*\n(.*?)\n```", re.DOTALL)
|
||||||
|
_ID_RE = re.compile(r"^id:\s*(\S+)", re.MULTILINE)
|
||||||
|
_UUID_RE = re.compile(r'state_hub_task_id:\s*"?([0-9a-f-]{36})"?')
|
||||||
|
|
||||||
|
|
||||||
|
@dataclass
|
||||||
|
class BackfillReport:
|
||||||
|
scanned_files: int = 0
|
||||||
|
pairs_found: int = 0
|
||||||
|
updated: int = 0
|
||||||
|
already_set: int = 0
|
||||||
|
conflicts: list[dict[str, str]] = field(default_factory=list)
|
||||||
|
unmatched_uuids: int = 0
|
||||||
|
|
||||||
|
def to_dict(self) -> dict[str, Any]:
|
||||||
|
return {
|
||||||
|
"schema": "state-hub.task-record-id-backfill.v1",
|
||||||
|
"scanned_files": self.scanned_files,
|
||||||
|
"pairs_found": self.pairs_found,
|
||||||
|
"updated": self.updated,
|
||||||
|
"already_set": self.already_set,
|
||||||
|
"unmatched_uuids": self.unmatched_uuids,
|
||||||
|
"conflicts": self.conflicts,
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
def collect_pairs(roots: list[Path]) -> tuple[dict[str, str], BackfillReport]:
|
||||||
|
"""Map projection UUID -> canonical record id, from workplan files."""
|
||||||
|
report = BackfillReport()
|
||||||
|
pairs: dict[str, str] = {}
|
||||||
|
for root in roots:
|
||||||
|
wp_dir = root / "workplans"
|
||||||
|
if not wp_dir.is_dir():
|
||||||
|
continue
|
||||||
|
for path in sorted(wp_dir.rglob("*.md")):
|
||||||
|
if path.name.startswith("."):
|
||||||
|
continue
|
||||||
|
try:
|
||||||
|
text = path.read_text(encoding="utf-8")
|
||||||
|
except (OSError, UnicodeDecodeError):
|
||||||
|
continue
|
||||||
|
report.scanned_files += 1
|
||||||
|
for block in _TASK_BLOCK_RE.finditer(text):
|
||||||
|
body = block.group(1)
|
||||||
|
rid = _ID_RE.search(body)
|
||||||
|
uid = _UUID_RE.search(body)
|
||||||
|
if not rid or not uid:
|
||||||
|
continue
|
||||||
|
record_id, task_uuid = rid.group(1).strip(), uid.group(1)
|
||||||
|
prior = pairs.get(task_uuid)
|
||||||
|
if prior and prior != record_id:
|
||||||
|
# One UUID claimed by two canonical ids: a duplicate
|
||||||
|
# registration. Recording it and skipping is the only safe
|
||||||
|
# option — picking one would fabricate an identity.
|
||||||
|
report.conflicts.append(
|
||||||
|
{"uuid": task_uuid, "first": prior, "second": record_id}
|
||||||
|
)
|
||||||
|
continue
|
||||||
|
pairs[task_uuid] = record_id
|
||||||
|
report.pairs_found = len(pairs)
|
||||||
|
return pairs, report
|
||||||
|
|
||||||
|
|
||||||
|
async def backfill_task_record_ids(
|
||||||
|
session: Any, roots: list[Path], *, dry_run: bool = True
|
||||||
|
) -> BackfillReport:
|
||||||
|
from sqlalchemy import select
|
||||||
|
|
||||||
|
from api.models.task import Task
|
||||||
|
|
||||||
|
pairs, report = collect_pairs(roots)
|
||||||
|
if not pairs:
|
||||||
|
return report
|
||||||
|
|
||||||
|
rows = list((await session.execute(select(Task))).scalars())
|
||||||
|
by_id = {str(r.id): r for r in rows}
|
||||||
|
for task_uuid, record_id in pairs.items():
|
||||||
|
row = by_id.get(task_uuid)
|
||||||
|
if row is None:
|
||||||
|
report.unmatched_uuids += 1
|
||||||
|
continue
|
||||||
|
if row.record_id == record_id:
|
||||||
|
report.already_set += 1
|
||||||
|
continue
|
||||||
|
if row.record_id and row.record_id != record_id:
|
||||||
|
report.conflicts.append(
|
||||||
|
{"uuid": task_uuid, "first": row.record_id, "second": record_id}
|
||||||
|
)
|
||||||
|
continue
|
||||||
|
report.updated += 1
|
||||||
|
if not dry_run:
|
||||||
|
row.record_id = record_id
|
||||||
|
return report
|
||||||
36
migrations/versions/e2b3c4d5f6a7_task_canonical_record_id.py
Normal file
36
migrations/versions/e2b3c4d5f6a7_task_canonical_record_id.py
Normal file
|
|
@ -0,0 +1,36 @@
|
||||||
|
"""task canonical record id (STATE-WP-0083-T06)
|
||||||
|
|
||||||
|
Every work-record type carries a stable canonical identifier except tasks. A
|
||||||
|
task row holds `id, workplan_id, title, status, priority` — nothing connecting it
|
||||||
|
to `CUST-WP-0067-T01` in the file it came from.
|
||||||
|
|
||||||
|
The consequence is that a file task and a hub task can only be matched by title,
|
||||||
|
so a renamed heading looks like one task disappearing and another appearing. That
|
||||||
|
is why the forge-derived reset deliberately refuses to touch tasks of existing
|
||||||
|
workplans: destroying and recreating a record because someone edited a heading is
|
||||||
|
not an acceptable failure mode.
|
||||||
|
|
||||||
|
Nullable on purpose. Existing rows have no canonical id and cannot be given one
|
||||||
|
by this migration — only the repository files know the mapping, and backfilling
|
||||||
|
from them is a separate, reversible step.
|
||||||
|
|
||||||
|
Revision ID: e2b3c4d5f6a7
|
||||||
|
Revises: d1a2b3c4e5f6
|
||||||
|
"""
|
||||||
|
from alembic import op
|
||||||
|
import sqlalchemy as sa
|
||||||
|
|
||||||
|
revision = "e2b3c4d5f6a7"
|
||||||
|
down_revision = "d1a2b3c4e5f6"
|
||||||
|
branch_labels = None
|
||||||
|
depends_on = None
|
||||||
|
|
||||||
|
|
||||||
|
def upgrade() -> None:
|
||||||
|
op.add_column("tasks", sa.Column("record_id", sa.String(length=120), nullable=True))
|
||||||
|
op.create_index("ix_tasks_record_id", "tasks", ["record_id"])
|
||||||
|
|
||||||
|
|
||||||
|
def downgrade() -> None:
|
||||||
|
op.drop_index("ix_tasks_record_id", table_name="tasks")
|
||||||
|
op.drop_column("tasks", "record_id")
|
||||||
123
tests/test_task_record_id_backfill.py
Normal file
123
tests/test_task_record_id_backfill.py
Normal file
|
|
@ -0,0 +1,123 @@
|
||||||
|
"""Recovering canonical identity for existing task rows (STATE-WP-0083-T06).
|
||||||
|
|
||||||
|
The mapping exists only in the repository files, where a task declares both its
|
||||||
|
canonical id and the projection UUID it was registered under. Reading the pairing
|
||||||
|
is safe; inferring it from titles is not, which is the whole reason this exists.
|
||||||
|
"""
|
||||||
|
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
from pathlib import Path
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
|
||||||
|
from api.services import task_record_id_backfill as bf
|
||||||
|
|
||||||
|
|
||||||
|
def _repo(tmp_path: Path, name: str, body: str) -> Path:
|
||||||
|
root = tmp_path / name
|
||||||
|
(root / "workplans").mkdir(parents=True)
|
||||||
|
(root / "workplans" / "WP.md").write_text(body, encoding="utf-8")
|
||||||
|
return root
|
||||||
|
|
||||||
|
|
||||||
|
def test_reads_the_pairing_a_file_declares(tmp_path):
|
||||||
|
root = _repo(tmp_path, "a", (
|
||||||
|
"---\nid: DEMO-WP-0001\ntype: workplan\n---\n\n"
|
||||||
|
'```task\nid: DEMO-WP-0001-T01\nstate_hub_task_id: "11111111-1111-5111-8111-111111111111"\n```\n'
|
||||||
|
))
|
||||||
|
pairs, rep = bf.collect_pairs([root])
|
||||||
|
assert pairs == {"11111111-1111-5111-8111-111111111111": "DEMO-WP-0001-T01"}
|
||||||
|
assert rep.pairs_found == 1
|
||||||
|
|
||||||
|
|
||||||
|
def test_task_without_a_projection_uuid_is_skipped(tmp_path):
|
||||||
|
"""No pairing to read means no identity to assign."""
|
||||||
|
root = _repo(tmp_path, "b", (
|
||||||
|
"---\nid: DEMO-WP-0001\ntype: workplan\n---\n\n"
|
||||||
|
"```task\nid: DEMO-WP-0001-T01\nstatus: todo\n```\n"
|
||||||
|
))
|
||||||
|
pairs, _ = bf.collect_pairs([root])
|
||||||
|
assert pairs == {}
|
||||||
|
|
||||||
|
|
||||||
|
def test_one_uuid_claimed_twice_is_a_conflict_not_a_guess(tmp_path):
|
||||||
|
"""Duplicate registration: picking a winner would fabricate an identity."""
|
||||||
|
dup = "22222222-2222-5222-8222-222222222222"
|
||||||
|
a = _repo(tmp_path, "a", (
|
||||||
|
"---\nid: A-WP-0001\ntype: workplan\n---\n\n"
|
||||||
|
f'```task\nid: A-WP-0001-T01\nstate_hub_task_id: "{dup}"\n```\n'
|
||||||
|
))
|
||||||
|
b = _repo(tmp_path, "b", (
|
||||||
|
"---\nid: B-WP-0001\ntype: workplan\n---\n\n"
|
||||||
|
f'```task\nid: B-WP-0001-T01\nstate_hub_task_id: "{dup}"\n```\n'
|
||||||
|
))
|
||||||
|
pairs, rep = bf.collect_pairs([a, b])
|
||||||
|
assert len(rep.conflicts) == 1
|
||||||
|
assert pairs[dup] == "A-WP-0001-T01" # first wins, second recorded not applied
|
||||||
|
|
||||||
|
|
||||||
|
class _Row:
|
||||||
|
def __init__(self, tid, record_id=None):
|
||||||
|
self.id = tid
|
||||||
|
self.record_id = record_id
|
||||||
|
|
||||||
|
|
||||||
|
class _Session:
|
||||||
|
def __init__(self, rows):
|
||||||
|
self.rows = rows
|
||||||
|
|
||||||
|
async def execute(self, *_a, **_k):
|
||||||
|
rows = self.rows
|
||||||
|
|
||||||
|
class R:
|
||||||
|
def scalars(self_inner):
|
||||||
|
return iter(rows)
|
||||||
|
|
||||||
|
return R()
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_dry_run_changes_nothing(tmp_path):
|
||||||
|
root = _repo(tmp_path, "a", (
|
||||||
|
"---\nid: DEMO-WP-0001\ntype: workplan\n---\n\n"
|
||||||
|
'```task\nid: DEMO-WP-0001-T01\nstate_hub_task_id: "11111111-1111-5111-8111-111111111111"\n```\n'
|
||||||
|
))
|
||||||
|
row = _Row("11111111-1111-5111-8111-111111111111")
|
||||||
|
rep = await bf.backfill_task_record_ids(_Session([row]), [root], dry_run=True)
|
||||||
|
assert rep.updated == 1 and row.record_id is None
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_applies_the_identity_when_not_dry_run(tmp_path):
|
||||||
|
root = _repo(tmp_path, "a", (
|
||||||
|
"---\nid: DEMO-WP-0001\ntype: workplan\n---\n\n"
|
||||||
|
'```task\nid: DEMO-WP-0001-T01\nstate_hub_task_id: "11111111-1111-5111-8111-111111111111"\n```\n'
|
||||||
|
))
|
||||||
|
row = _Row("11111111-1111-5111-8111-111111111111")
|
||||||
|
rep = await bf.backfill_task_record_ids(_Session([row]), [root], dry_run=False)
|
||||||
|
assert rep.updated == 1 and row.record_id == "DEMO-WP-0001-T01"
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_a_row_the_files_do_not_claim_keeps_no_identity(tmp_path):
|
||||||
|
"""Unknown must stay unknown; the reset then keeps refusing to act on it."""
|
||||||
|
root = _repo(tmp_path, "a", (
|
||||||
|
"---\nid: DEMO-WP-0001\ntype: workplan\n---\n\n"
|
||||||
|
'```task\nid: DEMO-WP-0001-T01\nstate_hub_task_id: "11111111-1111-5111-8111-111111111111"\n```\n'
|
||||||
|
))
|
||||||
|
orphan = _Row("99999999-9999-4999-8999-999999999999")
|
||||||
|
await bf.backfill_task_record_ids(_Session([orphan]), [root], dry_run=False)
|
||||||
|
assert orphan.record_id is None
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_existing_identity_is_never_overwritten(tmp_path):
|
||||||
|
root = _repo(tmp_path, "a", (
|
||||||
|
"---\nid: DEMO-WP-0001\ntype: workplan\n---\n\n"
|
||||||
|
'```task\nid: DEMO-WP-0001-T01\nstate_hub_task_id: "11111111-1111-5111-8111-111111111111"\n```\n'
|
||||||
|
))
|
||||||
|
row = _Row("11111111-1111-5111-8111-111111111111", record_id="OTHER-WP-0001-T09")
|
||||||
|
rep = await bf.backfill_task_record_ids(_Session([row]), [root], dry_run=False)
|
||||||
|
assert row.record_id == "OTHER-WP-0001-T09"
|
||||||
|
assert rep.conflicts and rep.updated == 0
|
||||||
|
|
@ -219,7 +219,7 @@ filesystem.
|
||||||
|
|
||||||
```task
|
```task
|
||||||
id: STATE-WP-0083-T06
|
id: STATE-WP-0083-T06
|
||||||
status: todo
|
status: progress
|
||||||
priority: high
|
priority: high
|
||||||
```
|
```
|
||||||
|
|
||||||
|
|
@ -239,6 +239,35 @@ Acceptance: task rows carry their canonical record id; the task diff for
|
||||||
`whitehat-security` reports clean; `the-custodian`'s task diff falls to something
|
`whitehat-security` reports clean; `the-custodian`'s task diff falls to something
|
||||||
manual inspection confirms.
|
manual inspection confirms.
|
||||||
|
|
||||||
|
**Built (2026-08-26).** Migration `e2b3c4d5f6a7` adds `tasks.record_id`,
|
||||||
|
nullable because no migration can invent an identity for an existing row — only
|
||||||
|
the repository files hold the mapping.
|
||||||
|
|
||||||
|
The backfill *reads* that mapping rather than inferring it: a file task declares
|
||||||
|
both its canonical id and the projection UUID it was registered under, so the
|
||||||
|
pairing is stated, not guessed. Across 121 repositories and 1104 files it
|
||||||
|
recovered **5516 pairs with zero conflicts**, and identified **4456 of 6073**
|
||||||
|
task rows in the cache database.
|
||||||
|
|
||||||
|
Diff and reset now key on `record_id` where present, falling back to a
|
||||||
|
`title:`-prefixed key so an unidentified row is *visibly* unidentified rather
|
||||||
|
than silently title-matched.
|
||||||
|
|
||||||
|
Two refusals are tested rather than documented: a row the files do not claim
|
||||||
|
keeps no identity, and a row that already has one is never overwritten — a
|
||||||
|
mismatch is recorded as a conflict, not resolved. Title matching would have
|
||||||
|
"worked" and destroyed a record every time someone edited a heading.
|
||||||
|
|
||||||
|
**Remaining, in order.** Deploy the migration to central; then run the backfill
|
||||||
|
against central as an explicit operation — deliberately *not* inside the Helm
|
||||||
|
hook, where a partial failure would silently leave half the tasks identified.
|
||||||
|
The cache matched 4456 of 6073 and central has a different history, so the
|
||||||
|
numbers will differ and a dry run should be compared before applying.
|
||||||
|
|
||||||
|
Only then may `T03` be extended to tasks of existing workplans, which is a
|
||||||
|
further change with its own verification. `CUST-WP-0068-T09` is therefore two
|
||||||
|
moves away, not one.
|
||||||
|
|
||||||
|
|
||||||
## Restore a migration mechanism for central
|
## Restore a migration mechanism for central
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue