fix(substantive_fixer): SQL-side actionable-tag filter, eliminate head-of-line
Step 4 of the stuck-PR triage. Push the FIXABLE/CONVERTIBLE/UNFIXABLE_TAGS intersection from a post-fetch Python loop into the SELECT WHERE clause via json_each + EXISTS. LIMIT 3 now always returns 3 actionable rows (or fewer if that's all there are), eliminating the head-of-line block where 3 oldest empty-eval_issues PRs occupied the slots forever. Background: 11 hours of post-deploy logs showed substantive_fix_cycle stuck emitting "0 actionable from 3 candidate(s) — head-of-line: [(3922, []), (3926, []), (3940, [])]" every cycle. Reaper closed those three on schedule, then a new triple of empty-eval_issues PRs took their place. Reaper-as-primary-clearance worked but is defense-in-depth, not the right architecture. Source of the block is upstream in this SELECT. Implementation choice: json_each + EXISTS over LIKE. Robust against tag-name substring overlap, future-proof against tag renames, and SQLite 3.45.1 on VPS fully supports it. Verified live: returns 13 of 28 currently-stuck PRs as actionable, 15 fall through to reaper as before. Tag list builds from the routing constants at runtime so adding a new tag auto-updates the SELECT filter — no two-place edit footgun. WARN-on-corrupt-JSON path retained as defense-in-depth (json_each and json.loads use different parsers; technically possible for a row to pass one but not the other). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
4b2b59b184
commit
5db6a0248c
1 changed files with 23 additions and 23 deletions
|
|
@ -522,53 +522,53 @@ async def substantive_fix_cycle(conn, max_workers=None) -> tuple[int, int]:
|
||||||
Finds PRs with substantive issue tags that haven't exceeded fix budget.
|
Finds PRs with substantive issue tags that haven't exceeded fix budget.
|
||||||
Processes up to 3 per cycle (Rhea: 180s interval, don't overwhelm eval).
|
Processes up to 3 per cycle (Rhea: 180s interval, don't overwhelm eval).
|
||||||
"""
|
"""
|
||||||
|
# Build the actionable-tag list from the routing constants so adding a new
|
||||||
|
# tag to FIXABLE_TAGS / CONVERTIBLE_TAGS / UNFIXABLE_TAGS auto-updates the
|
||||||
|
# SELECT filter — no two-place edit footgun.
|
||||||
|
actionable_tags = sorted(FIXABLE_TAGS | CONVERTIBLE_TAGS | UNFIXABLE_TAGS)
|
||||||
|
placeholders = ",".join(["?"] * len(actionable_tags))
|
||||||
|
|
||||||
|
# Push the actionable-tag filter into SQL (was a post-fetch Python loop).
|
||||||
|
# The old shape selected the 3 oldest request_changes PRs and then dropped
|
||||||
|
# ones without actionable tags, so empty-eval_issues rows occupied LIMIT-3
|
||||||
|
# forever (head-of-line). Now LIMIT-3 always returns 3 actionable rows.
|
||||||
|
# Reaper handles the empty-tag PRs after their 24h cooldown.
|
||||||
rows = conn.execute(
|
rows = conn.execute(
|
||||||
"""SELECT number, eval_issues FROM prs
|
f"""SELECT number, eval_issues FROM prs
|
||||||
WHERE status = 'open'
|
WHERE status = 'open'
|
||||||
AND tier0_pass = 1
|
AND tier0_pass = 1
|
||||||
AND (domain_verdict = 'request_changes' OR leo_verdict = 'request_changes')
|
AND (domain_verdict = 'request_changes' OR leo_verdict = 'request_changes')
|
||||||
AND COALESCE(fix_attempts, 0) < ?
|
AND COALESCE(fix_attempts, 0) < ?
|
||||||
AND (last_attempt IS NULL OR last_attempt < datetime('now', '-3 minutes'))
|
AND (last_attempt IS NULL OR last_attempt < datetime('now', '-3 minutes'))
|
||||||
|
AND EXISTS (
|
||||||
|
SELECT 1 FROM json_each(eval_issues)
|
||||||
|
WHERE value IN ({placeholders})
|
||||||
|
)
|
||||||
ORDER BY created_at ASC
|
ORDER BY created_at ASC
|
||||||
LIMIT 3""",
|
LIMIT 3""",
|
||||||
(MAX_SUBSTANTIVE_FIXES + config.MAX_FIX_ATTEMPTS,), # Total budget: mechanical + substantive
|
(MAX_SUBSTANTIVE_FIXES + config.MAX_FIX_ATTEMPTS, *actionable_tags),
|
||||||
).fetchall()
|
).fetchall()
|
||||||
|
|
||||||
if not rows:
|
if not rows:
|
||||||
return 0, 0
|
return 0, 0
|
||||||
|
|
||||||
# Filter to only PRs with substantive issues (not just mechanical)
|
# Defense-in-depth: corrupt eval_issues JSON shouldn't reach here (json_each
|
||||||
|
# would error in the SELECT and SQLite would skip the row), but the WARN log
|
||||||
|
# stays so we catch any edge case where a row's JSON parses for json_each
|
||||||
|
# but not for json.loads (different parsers, technically).
|
||||||
substantive_rows = []
|
substantive_rows = []
|
||||||
skipped_no_tags = []
|
|
||||||
for row in rows:
|
for row in rows:
|
||||||
try:
|
try:
|
||||||
issues = json.loads(row["eval_issues"] or "[]")
|
json.loads(row["eval_issues"] or "[]")
|
||||||
except (json.JSONDecodeError, TypeError):
|
except (json.JSONDecodeError, TypeError):
|
||||||
# Corrupt JSON in eval_issues is abnormal (post-merge column drift,
|
|
||||||
# hand-edited row, partial write during crash). WARN so ops can chase
|
|
||||||
# the upstream column-write path. Without this, the row drops out of
|
|
||||||
# both substantive_rows and skipped_no_tags — the third silent path.
|
|
||||||
logger.warning(
|
logger.warning(
|
||||||
"PR #%d: corrupt eval_issues JSON — skipping in substantive fix cycle",
|
"PR #%d: corrupt eval_issues JSON — skipping in substantive fix cycle",
|
||||||
row["number"],
|
row["number"],
|
||||||
)
|
)
|
||||||
continue
|
continue
|
||||||
if set(issues) & (FIXABLE_TAGS | CONVERTIBLE_TAGS | UNFIXABLE_TAGS):
|
|
||||||
substantive_rows.append(row)
|
substantive_rows.append(row)
|
||||||
else:
|
|
||||||
skipped_no_tags.append((row["number"], issues))
|
|
||||||
|
|
||||||
if not substantive_rows:
|
if not substantive_rows:
|
||||||
# Visibility for the LIMIT-3 head-of-line block: if the oldest
|
|
||||||
# candidates have no fixer-actionable tags (e.g. eval_issues=[],
|
|
||||||
# broken_wiki_links only), the cycle silently returns 0 — and the
|
|
||||||
# next cycle picks the same head-of-line, forever. Log the eval_issues
|
|
||||||
# of skipped candidates so the journal makes the block visible.
|
|
||||||
if skipped_no_tags:
|
|
||||||
logger.info(
|
|
||||||
"Substantive fix cycle: 0 actionable from %d candidate(s) — head-of-line: %s",
|
|
||||||
len(rows), skipped_no_tags,
|
|
||||||
)
|
|
||||||
return 0, 0
|
return 0, 0
|
||||||
|
|
||||||
fixed = 0
|
fixed = 0
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue