From 218f7fd8b574a080777ac2cd59d56e0f23164963 Mon Sep 17 00:00:00 2001 From: tegwick Date: Fri, 28 Aug 2026 15:01:58 +0200 Subject: [PATCH] fix(projection): rank competing claims instead of overwriting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two rows can claim one record. `cust-wp-0010` claimed CUST-WP-0010 by its own identifier while `workstream-lifecycle-documentation` claimed it by path, both pointing at that workplan's file; `kont-wp-0013` and `KONT-WP-0013-blob-storage-content-streaming` did the same. Assigning into `matched` unconditionally let whichever row came last win and dropped the other silently — never matched, so never stale, so never reported by any pass. It was also order-dependent, so which row survived depended on row iteration order. Claims are now ranked: derived UUID, then the record's own identifier, then backing path, then prefix heuristic. The strongest wins and the loser is displaced into `stale`, which makes it a retirement candidate rather than invisible. Ranking is order-independent, verified both ways round. Under ADR-007 the identifier is the identity, so the row naming the record wins and its stale title and path are repaired from the file; the hub-first row that never held the identifier retires with its history intact. 751 pass. Co-Authored-By: Claude Opus 5 Assistant: claude-code Assistant-Model: opus Assistant-Process: 2583210@bnt-lap001 Assistant-Session: f2bff2d5-e9b2-4338-92ca-10282a927006 --- api/services/forge_projection.py | 40 ++++++++++++++++++++++++++------ tests/test_forge_projection.py | 30 ++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 7 deletions(-) diff --git a/api/services/forge_projection.py b/api/services/forge_projection.py index 758cc0a..f0c0a48 100644 --- a/api/services/forge_projection.py +++ b/api/services/forge_projection.py @@ -720,17 +720,41 @@ async def reset_repository_projection( r.slug = _tombstone_slug(r.slug or str(r.id), r.projection_retired_at) outcome.released.append(before) + # How a row came to claim a key, strongest first. Two rows can claim the + # same record — `cust-wp-0010` by its own identifier and + # `workstream-lifecycle-documentation` by path, both pointing at + # CUST-WP-0010's file. Assigning into `matched` unconditionally let the + # later one win and dropped the other silently: never matched, so never + # stale, so never reported by any pass. + UUID_MATCH, SLUG_MATCH, PATH_MATCH, PREFIX_MATCH = 0, 1, 2, 3 + matched: dict[str, Any] = {} + claim: dict[str, int] = {} + displaced: list[Any] = [] + + def _claim(key: str, row: Any, strength: int) -> None: + held = claim.get(key) + if held is None: + matched[key], claim[key] = row, strength + return + if strength < held: + # The new claim is stronger; the incumbent loses the record and + # becomes a retirement candidate rather than disappearing. + displaced.append(matched[key]) + matched[key], claim[key] = row, strength + else: + displaced.append(row) + for row in rows: key = (row.slug or "").strip().lower() by_uuid = want_by_uuid.get(str(row.id)) if by_uuid is not None: - matched[by_uuid] = row + _claim(by_uuid, row, UUID_MATCH) continue if key in want: # The identifier itself derives again — including for a retired row, # which is then deliberately un-retired below. - matched[key] = row + _claim(key, row, SLUG_MATCH) continue if row.projection_retired_at is None and RETIRED_SLUG_MARK in (row.slug or ""): # A stamped slug with a cleared flag is a resurrected row: the mark @@ -782,10 +806,12 @@ async def reset_repository_projection( continue bp = row.backing_relative_path if bp and _path_key(bp) in want_paths: - key = want_paths[_path_key(bp)] - else: - cand = [k for k in want if key.startswith(k + "-")] - key = cand[0] if len(cand) == 1 else key + _claim(want_paths[_path_key(bp)], row, PATH_MATCH) + continue + cand = [k for k in want if key.startswith(k + "-")] + if len(cand) == 1: + _claim(cand[0], row, PREFIX_MATCH) + continue matched[key] = row # An identifier this repository would create may already belong to another @@ -866,7 +892,7 @@ async def reset_repository_projection( stale = [ r for k, r in matched.items() if k not in want and r.projection_retired_at is None - ] + ] + [r for r in displaced if r.projection_retired_at is None] if stale and not derived.retirement_eligible: # Acknowledgement cannot authorise this. The caller is confirming that # records which stopped deriving should be retired; here nothing has diff --git a/tests/test_forge_projection.py b/tests/test_forge_projection.py index f0cb801..63f7592 100644 --- a/tests/test_forge_projection.py +++ b/tests/test_forge_projection.py @@ -949,3 +949,33 @@ class TestRetirementIsNotUndoneByPath: zombie = src.index("projection_retired_at is None and RETIRED_SLUG_MARK in") tail = src[zombie:zombie + 700] assert "matched[key] = row" in tail, "a resurrected row must enter `matched`" + + +class TestCompetingClaims: + """Two rows can claim one record; the weaker must not vanish. + + `cust-wp-0010` claimed CUST-WP-0010 by its own identifier while + `workstream-lifecycle-documentation` claimed it by path. Assigning into + `matched` unconditionally let the later one win and dropped the other + silently — never matched, so never stale, so never reported by any pass. + """ + + def test_claims_are_ranked_not_overwritten(self): + import inspect + src = inspect.getsource(fp.reset_repository_projection) + assert "def _claim(" in src + assert "displaced.append" in src + + def test_a_displaced_row_becomes_stale(self): + import inspect + src = inspect.getsource(fp.reset_repository_projection) + stale_at = src.index("stale = [") + assert "displaced" in src[stale_at:stale_at + 400] + + def test_identifier_beats_path(self): + """A row naming the record outranks one that merely shares its file.""" + import inspect + src = inspect.getsource(fp.reset_repository_projection) + assert src.index("UUID_MATCH, SLUG_MATCH, PATH_MATCH, PREFIX_MATCH = 0, 1, 2, 3") > 0 + # Strength is ordered strongest-first, so a lower number wins. + assert "if strength < held:" in src