fix(claim): skip only renovate bot issues, log filtered candidates - #219
Conversation
select_candidates dropped every queue item whose title merely contained "renovate" (bare IGNORECASE substring), so issues *about* Renovate — a standing category of work in this repo — were never claimable. Both lanes went empty on 2026-08-23 with a non-empty queue because of this. Replace the substring with the same narrow criteria dispatch uses in isRenovateIssue (src/lib/issue-filters.ts): "dependency dashboard" / "renovate dashboard" title substrings, "update dep" / "update image" title prefixes, and the renovate/dependencies/automated labels. The queue already omits bot issues by default (includeRenovate=false); this guard only covers the gap where a dispatch version or config serves them. Every item skipped by a bridge-side filter now logs "candidate-skipped" with the issue number and reason, so an empty lane is distinguishable from a filtered one. status/ready, lane match, and the claimable/agentMatch check are unchanged. Fixes #216
|
Reviewed and this is good — approach is right and I'd merge it. Two nits, neither blocking, plus one thing worth recording because it looked like a risk and isn't. The The On the fixture: swapping item 7 from Two things I checked that could have bitten and did not. Structured skip reasons like Note for sequencing: this needs a release and a home-ops digest bump before the queue actually unblocks, since the deployed image still carries the old filter. #208 (0.7.1) is green and ahead of it. |
…at INFO Per review: lane-mismatch / not-ready / not-claimable skips are expected noise (dispatch filters lane/status server-side before the queue reaches the bridge) and would spam one INFO line per item per tick if select_candidates ever ran over an unfiltered queue. The bot-filter skips stay at INFO — that is the #216 failure mode a lane silently starved by a bridge-side filter. Adds coverage: the mechanical skip reasons log at DEBUG and are silent at the default INFO level.
|
Applied the log-level nit in On the Confirmed the two non-bites independently: On sequencing: noted. #208 (0.7.1) is green and ahead in the queue, and release-please will pick this up in the next release; the home-ops digest bump comes after that lands. |
There was a problem hiding this comment.
AI Automated Review (incremental)
Incremental review: reviewed the changes since the last managed review; unresolved findings from that review are carried forward.
Analysis engine: MiniMax-M2.7@https://litellm.jory.dev/v1 (anthropic) — primary route
Recommendation: Approve
This PR correctly fixes the Renovate filter regression (issue PR 216) by replacing a bare renovate substring match with the same narrow criteria dispatch uses in isRenovateIssue — dashboard substrings, update prefixes, and bot labels — so issues about Renovate are no longer starved from the claim queue.
Change-by-Change Findings
bridge/claim.py
- Renovate filter precision: The fix replaces the overly-broad substring check with narrowly-scoped criteria (
dependency dashboard/renovate dashboardsubstrings,update dep/update imageprefixes,renovate/dependencies/automatedlabels). This aligns exactly with dispatch'sisRenovateIssueand fixes the incident where two real issues (PR 55 and PR 199) were silently dropped on 2026-08-23. - Logging level separation (incremental delta): Bot-filter skips (the PR 216 failure mode) now log at
INFO, making them visible in operational logs. Mechanical skips (lane-mismatch,not-ready,not-claimable) log atDEBUGto avoid spamming INFO whenselect_candidatesreceives an unfiltered queue. The logiclogger.info if skip.startswith("renovate-") else logger.debugcorrectly routes each skip type. - Trade-off documentation: The comment acknowledging that the
dependencieslabel also filters hand-written issues is a good disclosure; if this becomes problematic, both dispatch and bridge would need coordinated changes.
tests/test_claim.py
test_mechanical_skips_log_at_debug_not_info(new): Correctly verifies that lane/status/claimable skips emit at DEBUG, not INFO, and that at INFO level they are silent. The test creates three items with distinct failure modes and asserts the correct(number, reason, levelno)tuples.test_skipped_candidate_is_logged_with_number_and_reason: Updated to assertrecords[0].levelno == logging.INFOfor the bot-filter skip, confirming the fix from the incremental delta.
tests/fixtures/dispatch_claim_sample.json
- Item PR 7 title changed from
"Update renovate bot config"(which should now be claimable) to"Update dependency pytest from 8.0.0 to 8.1.0"(a real bot title that should be filtered). This correctly reflects the new narrow criteria.
CI
- test: success
- docker: success
Verification
- The PR body correctly identifies the two starved issues (PR 55 and PR 199) from 2026-08-23 that motivated the fix.
- Repository history confirms commits
506b538(skip only renovate bot issues) ande4b08f4(log mechanical skips at DEBUG) as the two-fix sequence. - The
dependencieslabel trade-off is explicitly documented in code comments, meeting the requirement to name concrete files the fix touches.
Summary
select_candidatesdropped every queue item whose title merely contained"renovate" (a bare IGNORECASE substring), so issues about Renovate were never
claimable — both lanes logged
emptyon 2026-08-23 whileget_queuereturnedtwo
claimable: true,status/ready,lane: localitems:misospace/foreman-dispatch-bridge#55— "[P3] No requirements-dev.txt pin management — dev-only deps not managed by Renovate"misospace/llmkube-images#199— "[P3] Renovate config still groups coder apps that were consolidated into apps/llmkube-coder"Changes
bridge/claim.pyrenovatesubstring with the same narrow criteria dispatch uses inisRenovateIssue(src/lib/issue-filters.ts):dependency dashboard/renovate dashboardtitle substrings,update dep/update imagetitle prefixes, and therenovate/dependencies/automatedlabels. The queue already omits bot issues by default (includeRenovate=false); this guard only covers the gap where a dispatch version or config serves them.candidate-skippedwith the issue number and reason (also for lane/status/claimable skips), so an empty lane is distinguishable from a filtered one.status/ready, lane match, and theclaimable/agentMatchchecks are unchanged — nothing else is claimed that wasn't before.tests/fixtures/dispatch_claim_sample.json— item Renovate Dashboard 🤖 #7 is now a real bot title (Update dependency pytest from 8.0.0 to 8.1.0), since the old "update renovate bot config" title correctly becomes claimable under the new criteria.tests/test_claim.py— new coverage: a "renovate" mid-sentence title is yielded (both starved issue titles from the incident); a "Renovate Dashboard" / "Dependency Dashboard" title is not; an "Update dependency …" / "update image …" title is not; arenovatelabel is not; a skip emits a log record with number and reason.Verification
python -m pytest tests/ -q→ 503 passedpython -m mypy bridge/→ cleanFixes #216