feat(platform): board flow analysis from GitHub Projects V2 - #170
renatoguimaraescb wants to merge 6 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
lucastribioliclickbus
left a comment
There was a problem hiding this comment.
Li a PR inteira. A decisão de arquitetura (timeline events em vez de snapshot + diff) está certa e bem defendida, os gates de qualidade são o ponto mais forte, e a doc em docs/integrations/github-projects.md é excelente.
Deixei 3 comentários inline, todos no caminho de escrita — que é exatamente o que os dois checkboxes abertos dizem que ainda não rodou contra Postgres. Os três corrompem números que a tela mostra como confiáveis:
sync.ts:427—history_availablemarcado por intenção de buscar, não por resultado (e diverge do dry-run que validou o board real).board-flow.ts:245—flowEfficiencyconta zero para item sem transições, contra a regra 1 do próprio arquivo.client.ts:276— conteúdo inacessível viraDRAFT_ISSUE, e o gate reporta a causa errada ao usuário.
Tenho mais uma dúzia de achados de severidade média/baixa (paginação por OFFSET sem ordenação determinística, ausência de AbortSignal.timeout que o client Datadog tem, client.ts sem testes, entre outros) — te passo por fora para não poluir a thread, e você decide o que entra nesta PR e o que vira issue.
|
Vou deixar uns achados aqui e vc vê se faz sentido.
|
cbaefca to
5e2806f
Compare
Adds an ingestion + metrics module for GitHub Projects boards: lead time, time per column, throughput, WIP aging, CFD and bottleneck signals. The engine deliberately measures PR-open-to-merge and says nothing about the queue in front of it; board data is the missing denominator. If AI shortens coding but total lead time does not move, the constraint is outside the code — that is the question this makes answerable. No snapshot collector, and that is the design decision worth reviewing. Projects V2 exposes ProjectV2ItemStatusChangedEvent on the content's timeline, carrying createdAt, previousStatus, status and wasAutomated. Transition history is therefore readable retroactively at second precision on the first sync — no accumulation period, no +/-24h detection window, no blind spot for two moves inside one interval. Verified against a live board before writing any code. Structure follows the Datadog integration: a client, a syncOrganization that never throws, idempotent upserts keyed by the provider's own node ids, and a slot in the existing 04:00 UTC cron. Metrics live in lib/queries as pure functions, unit-tested like cycle-time-flow.ts. Quality gates run before any metric is trusted. Un-gated board data produced a median lead time of a fraction of a day on a real board — flattering and false, caused by setup cards created and closed minutes apart. Six gates report severity, the measured value, affected items and the impact on the reading; metrics still compute, but never without the caveat. Nothing encodes a particular workflow. Column names are free text, mapped to lifecycle buckets by per-board config first and generic EN/PT name heuristics second; unmatched columns are reported, never silently treated as not-done. Fixtures are fictional. Honesty rules that shaped the code: - lead time falls back transitions -> closedAt -> updatedAt, and anything past the first rung is labelled approximate; updatedAt is never used to invent a lead time for work still in flight - P95 withheld below 20 observations, everything but the median below 10 - re-entering a column accumulates both visits instead of overwriting - removal from the board is an exit, never a completion - drafts have no timeline on the API, so they are excluded from duration metrics rather than counted as zero, with coverage reported - assignee concentration describes the board, never a person: no login is returned, only the share (Principle #2) Little's Law is returned beside observed lead time, not instead of it — a large divergence points at phantom WIP or a mis-mapped terminal column. No UI in this change: the collector and the gates are the foundation, and metrics over unvalidated data are worth nothing. Dashboard follows once the numbers are validated against a real board. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…y-run
Adds scripts/board-flow-dryrun.ts — reads a real board through the real
client and runs the real gates and metrics without touching a database.
Ran it against a 198-item board with 754 status events (~14 GraphQL
requests, 17s), which surfaced three defects the unit tests could not.
1. Testing is real work. `synthetic_items` matched test/teste on their
own and flagged three genuine items ("Permitir teste de cenários de
rebooking", "Teste não moderado nova UI mobile") while catching zero
placeholders — a 100% false-positive rate on that path. Unambiguous
markers (dummy, asdf, lorem) still fire alone; ambiguous words now
need a short lifetime to corroborate.
2. Import is not scaffolding. 91 of the 198 items were created in
same-minute batches and closed minutes later — all real work,
imported when the board was set up. Same distortion as a test card
(near-zero lead time, throughput spike in one artificial week) but a
different remedy: you delete a placeholder, you exclude an import
from duration analysis. Split into its own `mass_import` gate so the
finding names what actually happened.
3. Renamed columns were counted twice. Per-phase stats keyed on the raw
name, so "Ready for Deploy" and "Ready for deploy" — one column,
renamed — split into two rows with a misleadingly small n each
(11 and 24 instead of 35). Now keyed on the normalized name, labelled
with the most frequent spelling. Genuinely different names for the
same stage stay separate; merging those needs explicit statusConfig,
since guessing would be wrong elsewhere.
Also excludes the terminal column from time-per-phase: an item sits in
Done until archived, so its time there measured age since delivery and
dominated the ranking at a 39-day median.
Regression tests added for each, including the three real titles that
were wrongly flagged.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ntry The ingestion module had no way in. Adds the route, the read layer it needed, and the nav entry. Read layer (lib/queries/board-flow-data.ts) loads items and events and hands them to the existing pure functions, so every calculation stays unit-testable without a database. Reads paginate explicitly: PostgREST caps responses at the project's max-rows, and a board above that would come back silently truncated — the failure mode behind #121. The route degrades instead of erroring. A missing schema (migration 023 not applied) and an org with no board both render an unconfigured state, so the nav entry can ship before the migration lands rather than 500ing on a deployment that hasn't migrated. Section order is the spec's and it is deliberate: quality gates before any number, so the reader knows what the figures can carry before reading them. Then durations, time per column, WIP aging, throughput, CFD, stalled items, Little's Law. Two visualization decisions: - The CFD groups by lifecycle bucket, not by column. The live board has 17 columns; stacking that many bands is unreadable, while five buckets make accumulation obvious. Needed the resolved bucket per column, so summarizeBoard now returns `statusBuckets`. - Ran the product's categorical ramp through a palette validator. It passes colour-vision separation (worst adjacent pair ΔE 18.6, target 8) but four of five slots fall below 3:1 contrast against the page surface. That obligates relief, so every mark is paired with a visible label or rendered as a table — identity is never colour alone. Same reason the gates lead with an icon and the severity word, not a dot. Nav entry lives in tenantNavItems, which the sidebar and the mobile sheet share, so one entry covers both. Translations in en-US and pt-BR; es-ES falls back to en-US per the existing convention. Not verified: the page has not been rendered against real data. The schema is not applied anywhere yet and this machine has no Supabase credentials, so only the empty and unconfigured states are reachable locally. Build and types pass; visual confirmation is still owed. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ion in board flow Two correctness gaps found in code review before merge: - resolveRepositoryId matched board items to Iris repositories by bare name only, ignoring the owner. A board spanning more than one GitHub owner (or two orgs sharing a repo name) could silently link an item to the wrong repository. Now matches on remote_url-derived "owner/repo" first, same fix the Datadog integration already made for the same reason, falling back to bare-name matching only for repos with no remote_url on file. - history_truncated was written by sync but never read: no query selected it, no type carried it, no gate surfaced it. An item whose timeline exceeded the pagination cap had its lead time reported as complete with no caveat. Wired into the history_coverage gate alongside history_available. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Addresses lucastribioliclickbus's review of #170 — all three corrupt numbers the UI shows as reliable: - sync.ts: markHistoryAvailable marked history_available=true for every item *attempted* this run, not just the ones that actually got a timeline back. A node coming back null (content deleted mid-sync) or without timelineItems left no entry in eventsByContentId, but was still marked available. Extracted the classification into a pure, now-tested classifyHistoryResults. - board-flow.ts: flowEfficiency divided activeHours (0 for an item with no transitions, by construction) by a lead time that can still resolve via the closedAt/updatedAt fallback ladder — reporting a real 0% instead of "unknown", against the file's own Rule 1. Now null unless there is at least one real visit. - client.ts: content: null (deleted, transferred, or access-revoked issue/PR) parsed as DRAFT_ISSUE by toContentType's fallback, so the history_coverage gate told the user "these are drafts" for items that are not drafts at all. Added a distinct UNKNOWN content type end to end (client, types, migration 023's CHECK, the gate's message). Also adds the test coverage gabriel-gomes-clickbus flagged as missing: client.ts's parseItem/toContentType (previously zero tests) and needsHistoryFetch, the pure logic behind the sync's daily-cost guarantee. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
5e2806f to
5559c60
Compare
|
Obrigado pelos achados — tratei cada um: 🟡 🟡 Os outros 3 (❌ conectar board, 🟠 isolamento do cron, 🟡 cache do CFD) não entraram nesta PR — nenhum bloqueia a corretude do que já está aqui, e resolver os 3 juntos ia inflar ainda mais uma PR que já está grande. Abri issues rastreando cada um, com o achado seu citado:
Se discordar da priorização ou quiser puxar algum desses para dentro desta PR, avisa. |
Tipo
Feature — novo módulo de ingestão + métricas + dashboard na plataforma.
Closes #252 (issue aberta retroativamente em 2026-09-16, registrando o que este PR já implementava — ver nota de processo no final).
Summary
Adiciona análise de fluxo de entrega a partir de GitHub Projects V2: lead time, tempo por coluna, throughput, WIP aging, CFD e sinais de gargalo — com portões de qualidade de dados na frente de tudo.
O motivo: o engine mede deliberadamente a janela PR-open → merge (
flow_efficiency.py) e não diz nada sobre a fila que vem antes. Se a IA encurta a fase de código mas o lead time total não se move, a restrição está fora do código. Esse é o denominador que faltava.Correção (2026-09-16): esta seção originalmente dizia "Sem UI nesta PR" — isso estava errado. O PR sempre carregou uma página de dashboard completa em
/[tenant]/flow(flow-view.tsx,flow-sections.tsx, entrada de menu). Ver "Por que a UI entra nesta PR" abaixo para a justificativa correta, exigida pelo non-goal de "dashboard sprawl" doCLAUDE.md.Por que a UI entra nesta PR
O
CLAUDE.mdproíbe "dashboard sprawl: each new view must tie to a validated insight, not a hypothetical user". Esta view não é hipotética: antes de qualquer linha de UI, o coletor + portões foram validados contra um board real de produção (198 itens, 754 eventos) e o dry-run (scripts/board-flow-dryrun.ts) encontrou e corrigiu três defeitos reais de classificação (ver "Validação contra board real" abaixo) — a métrica que a UI mostra já passou por essa validação, não é um número recém-calculado sendo exposto direto.A decisão de arquitetura que merece review
O desenho óbvio para "quanto tempo cada card ficou em cada coluna" é snapshot periódico + job de diff. Para Projects V2 isso é desnecessário.
A API expõe
ProjectV2ItemStatusChangedEventno timeline do conteúdo, comcreatedAt,previousStatus,statusewasAutomated. Validei contra um board real antes de escrever qualquer código:Consequências: histórico retroativo no primeiro sync (sem período de acumulação), precisão de segundo (sem janela de ±24h), CFD reconstruível para semanas passadas, e
wasAutomatedcomo sinal de movimentação automatizada melhor que heurística de timestamp.Único buraco:
DraftIssuenão éIssuee não temtimelineItems. Drafts ficam comhistory_available = falsee são excluídos de métricas de duração — nunca contados como zero.Como encaixa no que já existe
Espelha a integração Datadog:
client.ts+syncOrganization()que nunca lança + upsert idempotente pelos node ids do próprio provider + slot no cron diário que já roda às 04:00 UTC. Métricas emlib/queries/como funções puras, testadas comocycle-time-flow.ts.org_integrationsjá era genérica porprovider— só precisou do novo valor no enum.Tamanho do PR
5.868 linhas / 20 arquivos, acima da faixa de referência do time (RFC 0028 §2.4, que pede justificativa a partir de 251 linhas). Justificativa: coletor, portões de qualidade e migration são uma unidade só — uma métrica sobre dado sem portão de qualidade na frente já se provou enganosa neste mesmo board real (mediana de lead time de fração de dia, causada por cards de setup). Separar UI do coletor foi considerado (era a intenção original, daí o "sem UI" da primeira versão desta descrição) e descartado: a UI é o que torna a validação contra o board real verificável por quem não vai ler os testes, e adiá-la não reduz o tamanho do coletor, que já é a maior parte do diff.
Portões de qualidade
Não são seção secundária. Em board real, dado sem portão produziu mediana de lead time de fração de dia — lisonjeiro e falso, causado por cards de setup criados e fechados minutos depois.
synthetic_itemsmass_importdone_not_closedclosedAtcomo fallbackbulk_movementwasAutomateddo GitHubfield_completenessassignee_concentrationhistory_coverageAgnóstico de organização
Nada codifica um workflow específico. Nomes de coluna são texto livre, mapeados para buckets de ciclo de vida por config explícita por board e, na ausência dela, por heurísticas genéricas de nome (EN/PT). Coluna sem match é reportada, nunca tratada silenciosamente como não-terminal. Fixtures dos testes são fictícias.
Regras de honestidade implementadas
transitions → closedAt → updatedAt; tudo além do primeiro degrau viraapproximate.updatedAtnunca inventa lead time para trabalho em andamento.assignee_concentrationdescreve o board, nunca uma pessoa: não retorna login, só o percentual (Princípio build(deps): Bump actions/checkout from 4 to 6 #2) — confirmado por revisão de código e coberto por teste de regressão.Correções de revisão (2026-09-16)
Revisão linha a linha (Claude Sonnet 5) encontrou e corrigiu dois problemas de correção antes do merge:
resolveRepositoryIdcasava só pelo nome nu do repo, ignorando o owner. Um board que abrange mais de um owner GitHub (ou dois orgs com repo de mesmo nome) podia linkar um item ao repositório errado silenciosamente. Corrigido para casar por "owner/repo" derivado deremote_urlprimeiro — o mesmo fix que a integração Datadog já tinha feito pelo mesmo motivo — caindo para o nome nu só quando o repo não temremote_urlregistrado.history_truncatedera escrito e nunca lido. A migration e o sync já marcavam itens cujo timeline excedeu a paginação, mas nenhuma query selecionava a coluna, nenhum tipo a carregava, nenhum portão a checava. Um item com histórico muito longo tinha seu lead time reportado como completo sem ressalva. Agora entra no gatehistory_coverage.Nenhum problema de segurança encontrado (token nunca logado, criptografia reaproveita o padrão da migration 014 sem código novo, rota do cron com a mesma auth já existente, sem IDOR no
?board=da página — tudo já escopado por tenant no layout).Testes: 382/382 passam nesta rodada (era 324 na versão original desta descrição; a diferença inclui os 8 testes novos destas correções mais o que foi acumulado em
mainentretanto).tsc --noEmitlimpo.npm run lintsem erros novos.Correções de revisão humana (2026-09-16, segunda rodada)
gabriel-gomes-clickbuselucastribioliclickbusrevisaram a PR e deixaram achados reais — commit 5559c60:De
lucastribioliclickbus(3 comentários inline, todos no caminho de escrita):sync.ts:427—history_availablemarcado por intenção, não por resultado.markHistoryAvailablemarcava todo item tentado nesta rodada como disponível, mesmo quandofetchStatusHistorynão trouxe timeline de volta (conteúdo deletado/inacessível no meio do sync não deixa entrada emeventsByContentId). Extraída a classificação paraclassifyHistoryResults(pura, testada), que só marca disponível o que de fato voltou.board-flow.ts:245—flowEfficiencycontava zero para item sem transições.activeHours = 0para um item sem visitas é ausência de dado, não uma medição — masleadTimeHoursainda podia resolver via fallback declosedAt, produzindo umflowEfficiency: 0real onde a Regra 1 do próprio arquivo pedenull. Corrigido para exigir ao menos uma visita real.client.ts:276— conteúdo inacessível viravaDRAFT_ISSUE.content: null(issue/PR deletado, transferido, ou fora do escopo do token) caía no mesmo fallback de um draft de verdade, e o gatehistory_coveragedizia "são itens draft" para itens que não são drafts. Novo tipoUNKNOWN, distinto deDRAFT_ISSUE, propagado pelo client, pelos tipos, pela migration 023 (CHECK constraint) e pela mensagem do gate.De
gabriel-gomes-clickbus(5 achados, nenhum bloqueante):client.ts(558 linhas) sem nenhum teste → corrigido:tests/github-projects-client.test.tsnovo, cobrindoparseItem/toContentType(onde o bug build(deps): Bump actions/upload-artifact from 4 to 7 #3 acima vivia).needsHistoryFetch(a lógica que sustenta "custo diário proporcional à atividade do board") sem teste → corrigido: 6 casos novos emgithub-projects-sync.test.ts.settings/integrationssó lista Datadog) → issue [FEAT] UI para conectar um board GitHub Projects V2 #253, não resolvido nesta PR.computeCfdrecalculado a cada request SSR, sem cache → issue [DEBT] computeCfd recalculado a cada request SSR em /flow, sem cache #255, não resolvido nesta PR.Os três últimos não bloqueiam a corretude do que está aqui e resolver os três juntos inflaria ainda mais uma PR já grande — ficam rastreados para entrar depois deste merge.
Boas Práticas a Seguir
read:project), cifrado em repouso no padrão da migration 014syncOrganizationnão lança; gravalast_error)updatedAtmudouChecklist
client.ts/needsHistoryFetchpedidos pelo Gabrielnpx tsc --noEmitlimponpm run lintsem erros e sem warnings nos arquivos novosnpx vitest run— 401 passed (29 arquivos)npm run buildverde, incluindo a rota/[tenant]/flowscripts/board-flow-dryrun.ts— 198 itens, 754 eventos, ~14 requests, 17smain(2026-09-16, duas vezes — v1.7.0 e depois v1.8.0) — sem conflitos, suíte inteira revalidada em cadagabriel-gomes-clickbuselucastribioliclickbusrespondidos — 3 bugs corrigidos, 2 gaps de teste fechados, 3 achados de escala rastreados em [FEAT] UI para conectar um board GitHub Projects V2 #253/[DEBT] Cron sync-integrations sem isolamento de tempo por integração #254/[DEBT] computeCfd recalculado a cada request SSR em /flow, sem cache #255Platform (Next.js)só roda lint/tsc/vitest/build com env placeholder, sem Postgres real) nem este ambiente de revisão têm acesso a um Postgres real para validar isso; precisa de alguém com acesso a staging/Supabase antes do merge.Validação contra board real
scripts/board-flow-dryrun.tslê um board real pelo client real e roda gates e métricas reais, sem tocar banco. Rodado num board de 198 itens / 754 eventos (~14 requests GraphQL, 17s) — e achou três defeitos que os testes unitários não pegariam:test/testeisolados eram falso positivo. Flagravam 3 itens legítimos ("Permitir teste de cenários de rebooking") e zero placeholders — 100% de erro naquele caminho. Num board de engenharia, testar é o trabalho. Marcadores ambíguos agora exigem vida curta corroborando.Também: a coluna terminal saiu do tempo-por-fase —
Doneliderava com mediana de 39 dias, que é idade desde a entrega, não fluxo.O que ainda não foi validado: o caminho de escrita (migration + sync + upsert). Exige Docker/Supabase local, indisponível em todas as máquinas que já passaram por este PR até agora.
Autoria Assistida
gabriel-gomes-clickbuselucastribioliclickbus)lucastribioliclickbusmais a cobertura de teste pedida porgabriel-gomes-clickbus.lucastribioliclickbusrevisou a PR inteira e deixou 3 comentários inline, todos corrigidos e respondidos.gabriel-gomes-clickbusdeixou 5 achados de escala; 2 foram corrigidos (cobertura de teste) e 3 viraram issues ([FEAT] UI para conectar um board GitHub Projects V2 #253, [DEBT] Cron sync-integrations sem isolamento de tempo por integração #254, [DEBT] computeCfd recalculado a cada request SSR em /flow, sem cache #255) por serem escopo maior que este PR. Falta: confirmação do autor original (renatoguimaraescb) sobre a decisão de manter a UI nesta PR (ver "Por que a UI entra nesta PR") — uma chamada de escopo que cabe a quem definiu o escopo original, não à revisão automatizada.CLAUDE.mdpede; a issue formaliza o que já estava decidido e implementado aqui, no mesmo espírito do ADR retroativo que o PR [DEBT] CLAUDE.md out of date: Stage 3 opened in May; "external integrations" non-goal never amended #238 fez para a integração Datadog).Status
Os 3 bugs de caminho de escrita achados pela revisão humana estão corrigidos e respondidos. Falta: confirmação do autor original sobre a decisão de escopo da UI, e aplicar a migration em staging — os dois itens não marcados no checklist seguem bloqueando o merge.