Disagreement · 8ff8c1af-openai-1

Retention policy keeps RETENTION_LIMIT+1 rows (off-by-one)

mismatch

repo 56f59a0d·PR #2·reviewed 4 days ago

Primary finding

Retention policy keeps RETENTION_LIMIT+1 rows (off-by-one)

mediumbughigh

src/providers/telemetry.ts:206-214

Using id < (MAX(id) - LIMIT) retains rows with id >= (MAX - LIMIT), which is LIMIT+1 rows due to inclusive range (e.g., keep 50..1050 inclusive when MAX=1050, LIMIT=1000). If the intent is to keep exactly RETENTION_LIMIT rows, the condition should be id <= (MAX - LIMIT).

Recommendation

Use <= instead of <. Example: DELETE FROM routing_decisions WHERE id <= (SELECT MAX(id) - ? FROM routing_decisions); DELETE FROM failures WHERE id <= (SELECT MAX(id) - ? FROM failures); Bind RETENTION_LIMIT as a parameter to avoid string interpolation. Optionally, guard NULL MAX with COALESCE.

Counterpart finding

Telemetry enforceRetention deletes data from empty tables incorrectly

lowbugmedium

src/providers/telemetry.ts:207-219

When the table has fewer than RETENTION_LIMIT rows (e.g., 5 rows, MAX(id)=5), the subquery returns 5-1000=-995, so 'id < -995' matches nothing — fine. But when MAX(id) IS NULL (empty table), the subquery returns NULL and id < NULL is unknown, so no rows are matched — also fine. However, if AUTOINCREMENT ids have gaps (very common because failed inserts still consume ids), 'id < MAX(id)-1000' may delete MORE than necessary or delete recent rows that have low ids after a manual reset. More concerning: this scheme keeps the last 1001 rows by id, not by timestamp, which can keep stale entries if ids are reused after a vacuum. Minor maintainability issue.

Recommendation

Use ORDER BY timestamp DESC LIMIT RETENTION_LIMIT OFFSET 0 strategy, or DELETE FROM ... WHERE id NOT IN (SELECT id FROM ... ORDER BY id DESC LIMIT N).

Why this didn't post

This finding didn't meet AntFleet's unanimous agreement threshold. Both frontier models review every PR independently; only findings they both flag with the same severity and category are posted to the PR. This one fell through.

read the methodology →

From the same review

These findings passed the unanimous gate on the same PR review. The disagreement above was filtered out; the findings below were posted.

← back to all disagreements view public receipts see unanimous findings + anatomies →

Tweet ↗