Found while verifying #9136. That issue names the SQL join; this is a second instance of the same mismatch that it does not mention, and it is the one that silently disabled the recommender.
In computeCalibration (src/review/ops.ts):
const reverted = new Set((revRows.results ?? []).map((r) => r.target_id)); // review_audit → "owner/repo#123"
...
const isKept = !reverted.has(r.id); // review_targets → "project:kind:owner/repo#123"
The two id namespaces never intersect, so !reverted.has(...) was always true. Consequences, all structural rather than data-dependent:
- every merge read as "kept", so
rev was always empty
revertedMaxConfidence always null → recommendedFloor always null
- the calibration-drift Discord alert (
alerts.ts) could never fire
- the
/v1/internal/calibration bins showed a 100% kept-rate curve regardless of reality
This is not the same as the orphaned-table problem: even with review_targets fully repopulated, this comparison would still never match.
It was invisible because the test encoded the fiction
test/unit/ops.test.ts's calibrationEnv stub handed both queries the same bare ids ("a", "b", "c"), which made the namespaces agree in the test and only in the test. The suite passed for exactly as long as production was broken by their disagreement.
Fixing it means the fixture must use real owner/repo#n keys on both sides, so a future namespace regression fails the test rather than hiding inside it.
Found while verifying #9136. That issue names the SQL join; this is a second instance of the same mismatch that it does not mention, and it is the one that silently disabled the recommender.
In
computeCalibration(src/review/ops.ts):The two id namespaces never intersect, so
!reverted.has(...)was always true. Consequences, all structural rather than data-dependent:revwas always emptyrevertedMaxConfidencealways null →recommendedFlooralways nullalerts.ts) could never fire/v1/internal/calibrationbins showed a 100% kept-rate curve regardless of realityThis is not the same as the orphaned-table problem: even with
review_targetsfully repopulated, this comparison would still never match.It was invisible because the test encoded the fiction
test/unit/ops.test.ts'scalibrationEnvstub handed both queries the same bare ids ("a","b","c"), which made the namespaces agree in the test and only in the test. The suite passed for exactly as long as production was broken by their disagreement.Fixing it means the fixture must use real
owner/repo#nkeys on both sides, so a future namespace regression fails the test rather than hiding inside it.