Skip to content

Commit 8a9092b

Browse files
committed
fix(reconciler): global score-descending pairing prevents false-positive feature deactivations
The reconciler's matcher was per-write greedy first-fit: every synthesised write picked the best candidate that no earlier write had already claimed. With realistic inputs this routinely orphaned the candidate that would have been a clean pair for a later write while a borderline-match earlier write got the credit, then the orphaned candidate was swept inactive. Concrete prod evidence (ATOAPayment, 2026-06-01 PR-3220 merge): 28 features soft-deleted in one scan; at least 10 of them had a still-active twin with embedding cosine >= 0.85, several with identical titles -- "Cache Service" <-> "Cache Service" (0.857), "Vendor Rollout Flags" <-> "Vendor Rollout Flags" (0.845), "Upload Analytics Events" <-> "Analytics Upload Events" (0.853), "Pricing & Merchant Plans" <-> "Merchant Pricing & Plans" (0.876), and more. Replace _match_strategy (per-write greedy) with _resolve_pairings (global within-tier resolution). Tier order is preserved: signature -> Jaccard -> containment -> cosine. Within each tier the resolver collects every qualifying (write, candidate) edge, sorts descending by score with a stable tiebreak on (write_index, candidate_feature_id), and claims edges in that order. The strongest pair always wins. Sweep behavior, candidate_filter, deactivate_filter, audit-log contract, and the conservative absorb path are all unchanged. The change is algorithm-local; existing callers (synthesize.py full scan, pr_narrow_synthesis.py narrow path) need no changes and benefit equally. Validated locally by restoring the prod ATOAPayment snapshot and replaying the PR-3220 webhook through the backend's Redis-stream worker on the new matcher. High-confidence false-positives (cosine >= 0.85 twin) dropped from prod's 10+ to 4 on the local replay -- a meaningful incremental fix. The residual 4 are a cross-tier ordering pattern (a sibling write claims via signature/Jaccard while the better cosine pair is starved); a fuller fix would need global cross-tier optimization (Hungarian-style) and is out of scope for this change. Tests: - Updated test_feature_reconciler_containment.py's 4 unit tests to use the new _resolve_pairings entry via a _pair_one helper. Integration test untouched. - New test_feature_reconciler_global_pairing.py pins the regression with 6 cases: strongest-Jaccard-pair-wins regardless of input order; cosine score-descending claims correctly; signature tier still pre-empts lower tiers; sibling write does not block; tier order preserved when both Jaccard and cosine qualify for the same pair; unmatched candidate left for sweep. All 1818 backend tests pass; ruff and mypy --strict clean across 492 source files. Signed-off-by: Arun Rajkumar <mickyarunr@gmail.com>
1 parent cae1d83 commit 8a9092b

3 files changed

Lines changed: 419 additions & 120 deletions

File tree

backend/app/services/feature_reconciler.py

Lines changed: 133 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,13 @@
2828
preserved (and revivable on re-introduction) rather than silently
2929
disappearing.
3030
31+
Within each tier the resolver picks pairings globally by score-descending
32+
edge order, not by write-iteration order. That avoids the failure where
33+
an earlier write claims a candidate via a borderline match, leaving the
34+
later write that would have been a clean pair to insert-as-new while the
35+
displaced candidate gets swept inactive ("Cache Service ↔ Cache Service"
36+
deactivations seen in PR-merge synthesis on large repos).
37+
3138
The containment tier exists for the "narrow PR adds a sub-component to
3239
a broader feature" case: a 1-file PR re-synthesised on its own cluster
3340
emits a tiny feature whose files are a strict subset of an existing
@@ -122,9 +129,10 @@ async def reconcile_features_for_repo(
122129
123130
1. Bulk-load every existing feature for ``repo_id`` (active +
124131
inactive) so revival is single-pass.
125-
2. For each ``FeatureWrite``, run :func:`_match_strategy` to pick
126-
an existing row by signature → Jaccard → containment → cosine.
127-
First hit wins; ties broken by score.
132+
2. Call :func:`_resolve_pairings` to assign each ``FeatureWrite``
133+
to an existing row by signature → Jaccard → containment → cosine.
134+
Within each tier the highest-scoring edge claims its pair first
135+
(global resolution, not per-write greedy).
128136
3. If matched via signature / Jaccard / cosine: revive (when
129137
inactive) + ``update_in_place`` + refresh PRIMARY junction. If
130138
matched via containment: ``_absorb_into_existing`` — keep the
@@ -160,20 +168,19 @@ async def reconcile_features_for_repo(
160168
else all_candidates
161169
)
162170
by_signature: dict[str, ReconcilerCandidate] = {c.cluster_signature: c for c in candidates}
171+
pairings = _resolve_pairings(
172+
synthesised,
173+
candidates,
174+
by_signature,
175+
jaccard_threshold=jaccard_threshold,
176+
cosine_threshold=cosine_threshold,
177+
containment_threshold=containment_threshold,
178+
)
163179
matched_ids: set[uuid.UUID] = set()
164180
feat_repo = FeatureRepository(db, org_id=org_id)
165181
result = ReconcileResult()
166182

167-
for write in synthesised:
168-
match, match_via, score = _match_strategy(
169-
write,
170-
candidates,
171-
by_signature,
172-
matched_ids,
173-
jaccard_threshold=jaccard_threshold,
174-
cosine_threshold=cosine_threshold,
175-
containment_threshold=containment_threshold,
176-
)
183+
for write, (match, match_via, score) in zip(synthesised, pairings, strict=True):
177184
decision: str
178185
if match is None:
179186
await _insert_new(
@@ -371,83 +378,136 @@ async def _update_existing(
371378
)
372379

373380

374-
# O(n*m) — fine at hundreds; revisit at 5k+ features per repo.
375-
def _match_strategy(
376-
write: FeatureWrite,
381+
# O(n*m) per tier — fine at hundreds; revisit at 5k+ features per repo.
382+
def _resolve_pairings(
383+
synthesised: list[FeatureWrite],
377384
candidates: list[ReconcilerCandidate],
378385
by_signature: dict[str, ReconcilerCandidate],
379-
matched_ids: set[uuid.UUID],
380386
*,
381387
jaccard_threshold: float,
382388
cosine_threshold: float,
383389
containment_threshold: float,
384-
) -> tuple[ReconcilerCandidate | None, str, float]:
385-
"""Layered identity matcher: signature → Jaccard → containment → cosine.
386-
387-
Returns ``(candidate, match_via, score)`` where ``match_via`` is
388-
one of ``signature``, ``jaccard``, ``containment``, ``cosine``,
389-
``insert``. Score is 1.0 for an exact signature match, the
390-
Jaccard / containment / cosine value for the fallback tiers, and
391-
0.0 for ``insert``.
392-
393-
Containment is asymmetric — ``|write ∩ cand| / |write|`` — and only
394-
fires when the candidate is the larger side. It catches the case
395-
where a narrow PR re-synthesises a sub-cluster (a handful of files)
396-
whose files are already part of a broader existing feature; the
397-
earlier symmetric Jaccard tier misses this because the union
398-
blows up with the broader feature's wider footprint.
399-
400-
Skips candidates already claimed by a prior synthesised entry
401-
(``matched_ids``) so two synthesised features cannot collapse onto
402-
the same existing row.
390+
) -> list[tuple[ReconcilerCandidate | None, str, float]]:
391+
"""Globally resolve write→candidate pairings, tier-then-score order.
392+
393+
Tier priority is signature → Jaccard → containment → cosine. Within
394+
each tier the resolver claims the highest-scoring (write, candidate)
395+
edge first, then the next, until no candidate edges remain — instead
396+
of resolving each write in input order and letting the first write
397+
that walks the candidate list win.
398+
399+
The old per-write greedy left "Cache Service ↔ Cache Service" pairs
400+
unmatched whenever a sibling write reached the candidate first via a
401+
borderline Jaccard. The displaced candidate then had no match and
402+
was swept inactive. Score-descending within-tier resolution gives
403+
the strongest pair first dibs on the candidate.
404+
405+
Returns one ``(candidate, match_via, score)`` per synthesised entry,
406+
in input order. ``match_via=='insert'`` and ``candidate is None``
407+
when no tier claimed the write.
403408
"""
404-
sig_match = by_signature.get(write.cluster_signature)
405-
if sig_match is not None and sig_match.feature_id not in matched_ids:
406-
return sig_match, "signature", 1.0
407-
408-
write_paths = _flatten_paths(write.code_locations)
409-
if write_paths:
410-
best_jac: tuple[ReconcilerCandidate, float] | None = None
409+
n = len(synthesised)
410+
pairings: list[tuple[ReconcilerCandidate | None, str, float]] = [(None, "insert", 0.0)] * n
411+
claimed_writes: set[int] = set()
412+
claimed_candidates: set[uuid.UUID] = set()
413+
414+
# Tier 1: signature — exact cluster_signature, 1:1 by definition.
415+
for i, write in enumerate(synthesised):
416+
cand = by_signature.get(write.cluster_signature)
417+
if cand is not None and cand.feature_id not in claimed_candidates:
418+
pairings[i] = (cand, "signature", 1.0)
419+
claimed_writes.add(i)
420+
claimed_candidates.add(cand.feature_id)
421+
422+
# Memoise flattened paths so each side is built once across tiers.
423+
write_paths_memo: dict[int, set[str]] = {}
424+
cand_paths_memo: dict[uuid.UUID, set[str]] = {}
425+
426+
def write_paths(i: int) -> set[str]:
427+
cached = write_paths_memo.get(i)
428+
if cached is None:
429+
cached = _flatten_paths(synthesised[i].code_locations)
430+
write_paths_memo[i] = cached
431+
return cached
432+
433+
def cand_paths(c: ReconcilerCandidate) -> set[str]:
434+
cached = cand_paths_memo.get(c.feature_id)
435+
if cached is None:
436+
cached = _flatten_paths(c.code_locations)
437+
cand_paths_memo[c.feature_id] = cached
438+
return cached
439+
440+
def assign_tier(
441+
edges: list[tuple[float, int, ReconcilerCandidate]],
442+
label: str,
443+
) -> None:
444+
# Sort key, in order: score descending; lower write index wins
445+
# on ties (matches the old greedy by-iteration-order semantics);
446+
# candidate feature_id as the final deterministic tiebreak so
447+
# output never depends on the candidate-list order the upstream
448+
# loader happens to produce.
449+
edges.sort(key=lambda e: (-e[0], e[1], str(e[2].feature_id)))
450+
for score, i, cand in edges:
451+
if i in claimed_writes or cand.feature_id in claimed_candidates:
452+
continue
453+
pairings[i] = (cand, label, score)
454+
claimed_writes.add(i)
455+
claimed_candidates.add(cand.feature_id)
456+
457+
# Tier 2: Jaccard over file paths.
458+
jacc_edges: list[tuple[float, int, ReconcilerCandidate]] = []
459+
for i in range(n):
460+
if i in claimed_writes:
461+
continue
462+
wp = write_paths(i)
463+
if not wp:
464+
continue
411465
for cand in candidates:
412-
if cand.feature_id in matched_ids:
466+
if cand.feature_id in claimed_candidates:
413467
continue
414-
cand_paths = _flatten_paths(cand.code_locations)
415-
if not cand_paths:
468+
cp = cand_paths(cand)
469+
if not cp:
416470
continue
417-
score = _jaccard(write_paths, cand_paths)
418-
if score >= jaccard_threshold and (best_jac is None or score > best_jac[1]):
419-
best_jac = (cand, score)
420-
if best_jac is not None:
421-
return best_jac[0], "jaccard", best_jac[1]
422-
423-
best_cont: tuple[ReconcilerCandidate, float] | None = None
471+
score = _jaccard(wp, cp)
472+
if score >= jaccard_threshold:
473+
jacc_edges.append((score, i, cand))
474+
assign_tier(jacc_edges, "jaccard")
475+
476+
# Tier 3: asymmetric containment. Candidate must be strictly larger
477+
# — absorbing into a smaller candidate would truncate the synthesis
478+
# result, not the other way around.
479+
cont_edges: list[tuple[float, int, ReconcilerCandidate]] = []
480+
for i in range(n):
481+
if i in claimed_writes:
482+
continue
483+
wp = write_paths(i)
484+
if not wp:
485+
continue
424486
for cand in candidates:
425-
if cand.feature_id in matched_ids:
487+
if cand.feature_id in claimed_candidates:
426488
continue
427-
cand_paths = _flatten_paths(cand.code_locations)
428-
# Containment requires the candidate to be strictly larger —
429-
# absorbing into a smaller or equal-sized candidate would
430-
# truncate the synthesis result, not the other way around.
431-
if not cand_paths or len(write_paths) >= len(cand_paths):
489+
cp = cand_paths(cand)
490+
if not cp or len(wp) >= len(cp):
432491
continue
433-
score = _containment(write_paths, cand_paths)
434-
if score >= containment_threshold and (best_cont is None or score > best_cont[1]):
435-
best_cont = (cand, score)
436-
if best_cont is not None:
437-
return best_cont[0], "containment", best_cont[1]
438-
439-
if write.embedding:
440-
best_cos: tuple[ReconcilerCandidate, float] | None = None
492+
score = _containment(wp, cp)
493+
if score >= containment_threshold:
494+
cont_edges.append((score, i, cand))
495+
assign_tier(cont_edges, "containment")
496+
497+
# Tier 4: embedding cosine.
498+
cos_edges: list[tuple[float, int, ReconcilerCandidate]] = []
499+
for i, write in enumerate(synthesised):
500+
if i in claimed_writes or not write.embedding:
501+
continue
441502
for cand in candidates:
442-
if cand.feature_id in matched_ids or cand.embedding is None:
503+
if cand.feature_id in claimed_candidates or cand.embedding is None:
443504
continue
444505
score = _cosine(write.embedding, cand.embedding)
445-
if score >= cosine_threshold and (best_cos is None or score > best_cos[1]):
446-
best_cos = (cand, score)
447-
if best_cos is not None:
448-
return best_cos[0], "cosine", best_cos[1]
506+
if score >= cosine_threshold:
507+
cos_edges.append((score, i, cand))
508+
assign_tier(cos_edges, "cosine")
449509

450-
return None, "insert", 0.0
510+
return pairings
451511

452512

453513
def _flatten_paths(locations: dict[str, list[str]] | None) -> set[str]:

backend/tests/services/test_feature_reconciler_containment.py

Lines changed: 29 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
2424
Three properties under test:
2525
26-
1. ``_match_strategy`` returns ``match_via="containment"`` when the
26+
1. ``_resolve_pairings`` returns ``match_via="containment"`` when the
2727
synth's files are mostly inside a larger candidate.
2828
2. The containment tier does not fire when the synth side is the same
2929
size or larger than the candidate (no false absorbs into smaller
@@ -48,10 +48,32 @@
4848
JACCARD_THRESHOLD,
4949
FeatureWrite,
5050
ReconcilerCandidate,
51-
_match_strategy,
51+
_resolve_pairings,
5252
)
5353

5454

55+
def _pair_one(
56+
write: FeatureWrite,
57+
candidates: list[ReconcilerCandidate],
58+
) -> tuple[ReconcilerCandidate | None, str, float]:
59+
"""Run :func:`_resolve_pairings` for a single write and return its slot.
60+
61+
The matcher is now global (resolves an entire batch at once), so the
62+
single-write tier-behavior tests below wrap the write in a one-element
63+
list and read pairings[0].
64+
"""
65+
by_signature = {c.cluster_signature: c for c in candidates}
66+
pairings = _resolve_pairings(
67+
[write],
68+
candidates,
69+
by_signature,
70+
jaccard_threshold=JACCARD_THRESHOLD,
71+
cosine_threshold=COSINE_THRESHOLD,
72+
containment_threshold=CONTAINMENT_THRESHOLD,
73+
)
74+
return pairings[0]
75+
76+
5577
def _candidate(
5678
*,
5779
signature: str,
@@ -104,15 +126,7 @@ def test_containment_matches_when_synth_files_are_subset_of_larger_candidate() -
104126
)
105127
write = _write(signature="sig-narrow", files=["sidebar.vue"])
106128

107-
match, via, score = _match_strategy(
108-
write,
109-
[cand],
110-
by_signature={cand.cluster_signature: cand},
111-
matched_ids=set(),
112-
jaccard_threshold=JACCARD_THRESHOLD,
113-
cosine_threshold=COSINE_THRESHOLD,
114-
containment_threshold=CONTAINMENT_THRESHOLD,
115-
)
129+
match, via, score = _pair_one(write, [cand])
116130

117131
assert match is cand
118132
assert via == "containment"
@@ -130,15 +144,7 @@ def test_containment_skips_when_synth_is_not_smaller() -> None:
130144
cand = _candidate(signature="sig-cand", files=["a.vue", "b.vue"])
131145
write = _write(signature="sig-write", files=["a.vue", "c.vue"])
132146

133-
match, via, _score = _match_strategy(
134-
write,
135-
[cand],
136-
by_signature={cand.cluster_signature: cand},
137-
matched_ids=set(),
138-
jaccard_threshold=JACCARD_THRESHOLD,
139-
cosine_threshold=COSINE_THRESHOLD,
140-
containment_threshold=CONTAINMENT_THRESHOLD,
141-
)
147+
match, via, _score = _pair_one(write, [cand])
142148

143149
assert match is None
144150
assert via == "insert"
@@ -149,15 +155,7 @@ def test_signature_match_beats_containment() -> None:
149155
cand = _candidate(signature="sig-shared", files=["a.vue", "b.vue", "c.vue", "d.vue"])
150156
write = _write(signature="sig-shared", files=["a.vue"])
151157

152-
_match, via, score = _match_strategy(
153-
write,
154-
[cand],
155-
by_signature={cand.cluster_signature: cand},
156-
matched_ids=set(),
157-
jaccard_threshold=JACCARD_THRESHOLD,
158-
cosine_threshold=COSINE_THRESHOLD,
159-
containment_threshold=CONTAINMENT_THRESHOLD,
160-
)
158+
_match, via, score = _pair_one(write, [cand])
161159

162160
assert via == "signature"
163161
assert score == pytest.approx(1.0)
@@ -170,15 +168,7 @@ def test_jaccard_match_beats_containment() -> None:
170168
# Containment would also match (3/4 = 0.75) — but Jaccard runs first.
171169
write = _write(signature="sig-write", files=["a.vue", "b.vue", "c.vue", "d.vue"])
172170

173-
_match, via, _score = _match_strategy(
174-
write,
175-
[cand],
176-
by_signature={cand.cluster_signature: cand},
177-
matched_ids=set(),
178-
jaccard_threshold=JACCARD_THRESHOLD,
179-
cosine_threshold=COSINE_THRESHOLD,
180-
containment_threshold=CONTAINMENT_THRESHOLD,
181-
)
171+
_match, via, _score = _pair_one(write, [cand])
182172

183173
# Jaccard cares about |∩| / |∪|. Containment is asymmetric. The synth
184174
# is the LARGER side here, so containment is structurally disallowed
@@ -195,15 +185,7 @@ def test_containment_below_threshold_falls_through_to_insert() -> None:
195185
# 1 of 4 synth files in candidate → containment = 0.25 < 0.5.
196186
write = _write(signature="sig-write", files=["a.vue", "x.vue", "y.vue", "z.vue"])
197187

198-
match, via, _score = _match_strategy(
199-
write,
200-
[cand],
201-
by_signature={cand.cluster_signature: cand},
202-
matched_ids=set(),
203-
jaccard_threshold=JACCARD_THRESHOLD,
204-
cosine_threshold=COSINE_THRESHOLD,
205-
containment_threshold=CONTAINMENT_THRESHOLD,
206-
)
188+
match, via, _score = _pair_one(write, [cand])
207189

208190
assert match is None
209191
assert via == "insert"

0 commit comments

Comments
 (0)