Skip to content

fix(reconciler): global score-descending pairing prevents false-positive feature deactivations - #188

Merged
mickyarun merged 1 commit into
mainfrom
fix/reconciler-global-pairing
Jun 2, 2026
Merged

fix(reconciler): global score-descending pairing prevents false-positive feature deactivations#188
mickyarun merged 1 commit into
mainfrom
fix/reconciler-global-pairing

Conversation

@mickyarun

@mickyarun mickyarun commented Jun 2, 2026

Copy link
Copy Markdown
Owner

Summary

  • Replace the reconciler's per-write greedy first-fit matcher with a global within-tier resolver. Tier order preserved (signature → Jaccard → containment → cosine); within each tier the strongest (write, candidate) edge wins, regardless of write input order. The new function is _resolve_pairings in backend/app/services/feature_reconciler.py; the old _match_strategy is removed.
  • 4 existing reconciler unit tests refactored to call the new entry point via a one-line _pair_one helper. The integration test is untouched.
  • New test_feature_reconciler_global_pairing.py with 6 regression cases pinning the new behavior.

Why

On the 2026-06-01 merge to main of a large payments-domain repo, 28 features were soft-deleted in one scan. Of those, at least 10 had a still-active twin with embedding cosine ≥ 0.85 — several with literally 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)
  • Rapyd Auth Interceptor ↔ Rapyd Auth Interceptor (0.965)
  • Receipt Generation ↔ Receipts & PDF Generation (0.936)
  • Request Queue Service ↔ Request Queue Service (0.922)
  • Scheduled Tasks (Cron) ↔ Scheduled Tasks / Cron (0.920)
  • Merchant Wallet ↔ Wallet (0.886)
  • Format & Mask Utilities ↔ String Masking & Formatting Utilities (0.879)

Root cause: the per-write greedy let an earlier write claim a candidate via a borderline match (often Jaccard ≈ 0.7-0.8), starving the later write that would have been a clean cosine pair. The displaced candidate then had no match and was swept inactive at the end of reconcile. Write iteration order — i.e., LLM output order — silently determined which features survived.

Stage 1 (PR #185) removed BUD-closure as one entry point into this failure. The full-scan path (called from PR-merge above the narrow cap, and from any explicit rescan) still hit this matcher defect on every run. This PR fixes the algorithm.

What changed

  1. _resolve_pairings runs four passes — signature first as a 1:1 lookup, then Jaccard / containment / cosine each as a tier-local edge sort. For each non-signature tier:
    • Collect every (write, candidate) edge whose score clears the tier's threshold.
    • Sort descending by (score, -write_index, candidate_feature_id_str) — strongest first, lower write index wins on equal score (matches old greedy iteration-order semantics), candidate UUID as final deterministic tiebreak so behavior never depends on the candidate-list iteration order.
    • Claim edges greedily: assign the pair if both sides are unclaimed.
  2. Flattened path sets are memoised so each side is built at most once across tiers — strict perf improvement over the old per-tier rebuilds.
  3. reconcile_features_for_repo consumes the pre-computed pairings list and the rest of the CRUD path (_insert_new, _update_existing, _absorb_into_existing, sweep) is byte-identical.

The PR is algorithm-local: no caller signatures change, candidate_filter / deactivate_filter / feature_match_log semantics are preserved.

Test plan

  • ruff check backend/ — zero issues.
  • ruff format --check on changed files — clean.
  • mypy backend/app/ — 492 source files, no issues.
  • pytest backend/tests/ — 1818 passed.
  • pr-review-toolkit:code-reviewer subagent — addressed both findings (tiebreak determinism: added str(cand.feature_id) as third sort key; coverage gaps: added two extra regression tests for cross-write contention and tier ordering).
  • Local end-to-end replay: restored the prod snapshot of that repo (the local prod-data snapshot, 97 active features, alembic migrations brought to current head), replayed the merge webhook payload through the backend's Redis-stream pr_merge_worker so the in-process MCP token authenticates correctly. With the fix applied: 29 features deactivated; only 4 had a high-cosine twin still active — vs the prod baseline of 10+ under the old matcher. Material improvement (≈ 60% reduction in high-confidence false-positives in a single PR's replay).
  • Post-merge prod observability: monitor feature_match_log and features.is_active=false transitions on the next several PR-merge scans; expect the visible "Deactivated" rate on feature cards to drop materially.

Known follow-up (not blocking this PR)

The 4 residual false-positives in the local replay all have the same shape: a different new write claims the candidate via signature or Jaccard (tier 1 or 2), so the cosine-equivalent new write that would have been a better semantic pair has nothing left to claim. The within-tier global resolution doesn't address cross-tier ordering. A fuller fix would be Hungarian-style global optimization across all tiers (best edge overall wins, regardless of tier); that's a larger algorithm change and belongs in a separate PR if the residual rate stays elevated.

…ive 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>
@mickyarun
mickyarun merged commit d845694 into main Jun 2, 2026
16 checks passed
@mickyarun
mickyarun deleted the fix/reconciler-global-pairing branch June 2, 2026 14:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant