Repository navigation
fix(engine): gate degradation per PR state and retry transient errors - #290
Merged
cpenaforte-clickbus merged 10 commits intoOct 8, 2026
Conversation
The enrichment retry waited 2 s on any gh failure, deterministic ones included: a 401, a 404 or a GraphQL error fails the same way on the second try, so every run paid the pause per page and per PR state for nothing. The retry now reads gh's stderr and only tries again on a transient error (HTTP 5xx, a timeout, a dropped connection). The reviews pass, which the code documents as prone to 504s, had no retry at all, while one failure of it now drops review metrics, the flow block and staleness from the report. It gets the same single transient-only retry. The basic list stays as it was. The pause constant becomes _RETRY_DELAY_S, since both passes share it. Refs #236 Assisted-by: Claude Code <noreply@anthropic.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The PR read fetches merged, closed and open PRs separately, but the aggregator gated on the union of the three: a reviews or enrichment failure only for open PRs wiped review, flow, acceptance and merge strategy, which read merged PRs alone. That threw away good data. PullRequestFetch now keeps the failed steps per state (degraded_by_state), the window-cache slice inherits them, and every aggregate() call in the CLI and the org runner passes them. Review, flow, acceptance and merge strategy follow the merged state; open-PR staleness follows the open state; a closed-state failure omits nothing, since closed PRs only feed counts. Without the per-state map (a fetch that never reached the states, or no map at all) every state takes the union, as before. The origin funnel cannot tell the states apart from the union alone, so it now skips when enrichment failed and acceptance is absent. The aggregator always omits acceptance when the merged state's enrichment failed, so the funnel's "In PR" stage still never falls back to a made-up "every commit is in a PR" then, and the funnel survives a failure that hit only open or closed PRs. pr_enrichment_degraded keeps listing the union, so the JSON contract does not change, and a clean run's outputs stay identical. The report caveat and METRICS.md now say the omissions follow the PR state. Refs #236 Assisted-by: Claude Code <noreply@anthropic.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
When `gh pr list` failed for one state while the others were read, the PR list was not empty, so everything the missing state feeds read as a real zero: with the merged list gone, acceptance showed 0% for every group, the activity timeline 0 PRs merged a week, and merge strategy "unknown" with reliable commit metrics; with any list gone, Flow Load undercounted the WIP. A missing merged list now omits what merged PRs feed (PR lifecycle, review, Flow Efficiency, human coverage, acceptance, merge strategy) and leaves the timeline's prs_merged unknown; a missing open list omits every open-PR field; a missing list in any state omits Flow Load. Without the per-state map the union applies, and `fetch` counts as every list failed. Clean runs are unchanged. Refs #236 Assisted-by: Claude Code <noreply@anthropic.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The transient-error check matched anywhere in gh's stderr: "EOF" matched "thereof", and "timeout" matched any error about a repo named like timeout-svc, so a deterministic failure there was retried with the pause on every page. The repo's owner/name, name and owner are now blanked out of stderr first (only where they stand alone, so a repo named "out" leaves "timeout" whole), and the patterns match whole words: HTTP 5xx, timeout/timed out, deadline exceeded, connection reset/refused, EOF. GitHub's GraphQL "may be the result of a timeout" error is still retried. Also refresh the enrichment docstring, which still said a page failed "even after one retry". Refs #236 Assisted-by: Claude Code <noreply@anthropic.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The nullable-when cells said "`reviews` degraded" or "`enrichment` degraded" while the prose already gated each field on the PR state that feeds it; the cells now carry that state, and the ones a failed list omits say so (acceptance, funnel, timeline, PR lifecycle, Flow Load, Flow Efficiency, coverage, open PR aging, merge strategy). §15 documents the failed-list rule, defines a stopped enrichment pass as failed even when earlier pages came back, and says a 4xx or other GraphQL error is not retried (GitHub's "may be the result of a timeout" error is). Refs #236 Assisted-by: Claude Code <noreply@anthropic.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The report caveat listed flow, review and in-PR metrics as what a
degraded read may leave missing, but a failed open list, or a failed
enrichment or reviews pass for open PRs, omits open-PR fields too. The
caveat now names them ("open-PR metrics" / "de PRs abertos"); its
opening sentence is unchanged.
Refs #236
Assisted-by: Claude Code <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
§24 listed a failed list only in the nullable-when cell. It now says the whole series goes when `gh pr list` failed for any state (or `fetch`), author_concurrency included, and why: the missing list would undercount the WIP in every bucket it overlaps, and keeping only the commit-based author count would emit buckets in a shape the platform does not read. Refs #236 Assisted-by: Claude Code <noreply@anthropic.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Several notes still described the older PR read. The aggregate() docstring called `prs` merged PRs, but they come in all three states. METRICS.md §10 and the funnel module said Iris only fetches merged PRs; it reads merged, closed and open PRs, and the limitation is that acceptance counts only commits in merged PRs. §10 now says a failure confined to closed or open PRs leaves acceptance and the funnel where a clean run would have them, without implying they are always present. §15 now says that with merged enrichment failed, Flow Efficiency is computed over the PRs whose commit refs came back, so it is skewed rather than missing. Refs #236 Assisted-by: Claude Code <noreply@anthropic.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A failed merged list omits the PR lifecycle fields (counts, time to merge, cycle time), and a partial read skews them, but the report caveat did not list them among what may be missing or skewed. It now does, in both languages; its opening sentence is unchanged. Refs #236 Assisted-by: Claude Code <noreply@anthropic.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
cpenaforte-clickbus
added
type: tech-debt
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This was referenced Oct 8, 2026
…-list fix(engine): omit what a failed PR list feeds, per state
cpenaforte-clickbus
merged commit Oct 8, 2026
3a9cdfd
into
fix/pr-read-degradation-accuracy
4 of 5 checks passed
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Contexto e Motivação
A leitura de PRs busca três estados — merged, closed e open — e cada um pode degradar sozinho. A série tratava a degradação como a união dos três: uma falha só nos PRs abertos apagava métricas que vêm só dos mergeados, jogando fora dado bom. Além disso, a nova tentativa de 2 s valia para qualquer erro, inclusive os determinísticos (4xx, erro de GraphQL), que falham de novo; e a passada de reviews, que o próprio código documenta como sujeita a 504, não tinha nova tentativa nenhuma — e uma falha nela agora omite três seções do relatório.
Refs #236
Mudanças
_run_gh_with_one_retrysó tenta de novo quando a falha parece transitória (HTTP 5xx, timeout, conexão); erro determinístico volta direto, sem espera._ENRICHMENT_RETRY_DELAY_Svira_RETRY_DELAY_S, compartilhado.PullRequestFetch.degraded_by_stateguarda os passos degradados de cada estado; a fatia do cache multi-janela herda.degradedcontinua sendo a união, e é ela que vai parapr_enrichment_degradedno JSON — contrato inalterado.aggregate(..., pr_fetch_degraded_by_state=...)decide por estado: merge strategy, review, cobertura humana, flow, acceptance e funil olham o estado merged; PR aberto parado olha o open; o closed não apaga nada. Sem a informação por estado, vale a união, como antes. As oito chamadas deaggregate()emcli.pyeorg_runner.pypassam a informação.Plano de Teste
pytest tests/test_pr_fetch_degradation.py tests/test_window_cache.py -v— 504 recuperado na passada de reviews; 401, 404 e erro de GraphQL sem espera e sem nova tentativa; reviews degradado só no estado open mantendo as métricas dos merged; enriquecimento degradado só no closed mantendo merge strategy, flow e acceptance; a fatia do cache herdando o dict; as oito chamadas recebendo a informação — rodado em54c7a1d: 79 passedpytest tests/ -q— a suíte inteira, com o portão de determinismo da [FEAT] Comparador de paridade do metrics.json, campo a campo #214 — rodado em54c7a1d: local: 423 passed; a única falha é atest_status_reflects_state([BUG] test_agent_settings_hook lê o spool real — falha em máquina com registros pendentes #146), que só ocorre fora do CI, e otest_hook_binders.pyficou de fora por depender de rede; no CI o comando literal passaA verificação roda fora da máquina de quem construiu: o job
clido CI (RFC 0029, PM-4).Impacto e Risco
pr_enrichment_degradedcontinua igual; um run sem degradação sai byte a byte idêntico. Runs com falha restrita a um estado passam a manter as métricas dos outros estados.Sobre o tamanho (RFC 0028 §2.4): 411 linhas, faixa 251–500. A nova tentativa e a degradação por estado estão em commits separados; a extensão da regra para listas que falham foi para o PR seguinte para caber.
Plano de Rollback
git revertdo merge. A degradação volta a ser a união, e a nova tentativa volta a valer para qualquer erro; nada a migrar.Referências
Autoria Assistida
🤖 Generated with Claude Code