Corrigir os 20 achados dos reviews (ultrareview + code-review max) - #2
Merged
Merged
Conversation
Consolida os 20 achados únicos do ultrareview sobre o PR #1 e do /code-review max sobre backend/src, priorizados por impacto real. Serve de referência para separar bug pré-existente de regressão futura durante a reformulação arquitetural. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Pmgii3S5sfMZghFqQXhbkH
P1.1 — Fila travava para sempre com diagnóstico especial
Saudável e Fitotoxicidade são gravados com doenca_id nulo, mas o schema
exigia uuid. O .parse() estourava no primeiro item inválido e devolvia 400
para o lote inteiro, derrubando até 49 diagnósticos válidos. Como o erro
era anterior ao loop por item, eles nunca viravam FAILED: ficavam PENDING
indefinidamente e a fila nunca mais drenava.
- doenca_id agora é nullable no schema e na coluna do Postgres. Descartar
esses diagnósticos jogaria fora dado real de campo, que é justamente o
que o RF05 existe para preservar. O modelo já era assimétrico: llmDoencaId
sempre foi nullable.
- cada item é validado individualmente; um malformado vira failed_item em
vez de derrubar os que viajam com ele.
P1.3 — Veredito de cross-validation era forjável
O INSERT gravava crossValidationStatus e llm_* direto do cliente, sem o
guard que a branch de UPDATE tinha. Um cliente podia POSTar CONFIRMED com
texto arbitrário e o /diagnosis/cross-validate devolvia isso como segunda
opinião, sem carregar imagem nem chamar o LLM — burlando a invariante de
nunca ocultar divergência e contaminando o loop de feedback do RF06.
Status terminal agora só nasce do pipeline do servidor.
P2.3 — image_s3_key não era revalidado contra o prefixo do usuário
P2.4 — gate de catálogo passou a valer também na branch de UPDATE
P2.5 — timestamp malformado virava RangeError e travava a fila em retry
P2.6 — llm_doenca_id é foreign key e não era validado no catálogo
Testes: +7 (46 -> 53). Fixtures de sync passaram a usar o userId real,
já que a chave S3 agora precisa pertencer ao usuário autenticado.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Pmgii3S5sfMZghFqQXhbkH
…e erros
P1.2 — Nenhum limite por rota estava ativo
Os cinco limites eram passados como `config` no app.register(), que é opção
do plugin e não da rota. O @fastify/rate-limit lê de routeOptions.config,
preenchido só na definição da rota, então tudo rodava no global de 60/min:
login sem proteção reforçada contra força bruta, e o endpoint de
cross-validation (S3 + LLM multimodal, 15s) a 3x o volume planejado.
Teste reproduz o defeito: antes da correção o header x-ratelimit-limit
respondia '60' em todas as rotas.
Bug latente revelado pela correção: o errorResponseBuilder devolvia um
objeto sem statusCode, então exceder o limite virava 500 genérico em vez de
429. Ficou invisível enquanto os limites nunca disparavam.
P3.3 — Balde era por IP, dimensão errada para endpoint pago por usuário
Atrás de proxy toda a base dividia um balde; sem proxy, um aparelho em rede
móvel troca de IP e escapa do teto. O keyGenerator agora chaveia pelo usuário
autenticado. Como o plugin roda em onRequest, antes do preHandler que popula
request.user, a verificação do token acontece no próprio keyGenerator — do
contrário a chave por usuário nunca sairia do papel.
P3.8 — Log de erro não permitia localizar a falha
Registrava só name e code; um TypeError em produção virava
{errorName:'TypeError'} sem stack. Além disso o bloco redact era incompleto:
o wildcard `*` do pino casa exatamente um nível, então o segredo no topo e o
aninhado em dois níveis vazavam em texto claro.
Limitação agora desligável por RATE_LIMIT_ENABLED, ligada por padrão. As
suítes de integração exercitam num minuto muito mais requisições do que um
produtor real; o teste dedicado religa.
Testes: +7 (53 -> 60).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Pmgii3S5sfMZghFqQXhbkH
P1.4 — Carga de imagem do S3 sem teto de tamanho
O loader bufferizava o objeto inteiro e ainda gerava um base64 ~33% maior,
sem cap de ContentLength. Como o presigned PUT não impõe content-length-range,
o tamanho é controlado pelo cliente: poucas centenas de MB esgotavam a heap.
Combinado com o rate limit inerte (corrigido no commit anterior), era DoS
trivial. Teto de 10MB, o mesmo que o app já valida antes de enviar.
P2.2 — Falha deixava PENDING e o retry re-cobrava indefinidamente
Timeout, erro de provider e JSON ilegível retornavam antes do saveResult.
PENDING não é terminal, então o guard de idempotência não barrava e o cliente
offline-first re-tentava sem teto, cada ciclo custando um GetObject no S3 mais
uma inferência de visão paga. Agora persiste SKIPPED, que é o mesmo estado que
o app já usa quando a segunda opinião falha.
P3.1 — extractJson quebrava com prosa depois do JSON
A varredura primeiro-{ / último-} engolia qualquer chave em texto após o
objeto. Atinge o Claude, que só é instruído em prosa a responder JSON,
enquanto o Gemini força responseMimeType: application/json. Um veredito
válido virava 502 e a inferência paga era descartada. Substituída por
varredura balanceada que respeita strings, com blocos cercados.
P3.2 — createPending podia devolver 500 em vez do 200 idempotente
mobile_local_id é único globalmente, mas findDiagnostic filtra por
(userId, mobileLocalId). Num duplo toque ou retry dentro da janela de 15s do
LLM os dois lookups erram e o segundo insert colide com 23505 cru.
P3.6 — Repetição idempotente perdia o nome da doença fora do catálogo
Nova coluna llm_doenca_nome. Sem ela a primeira chamada devolvia a string do
LLM e a repetição devolvia null, escondendo a divergência — justamente o que
a RF08 proíbe.
P3.7 — Coordenadas (0,0) eram indistinguíveis de leitura real
(0,0) é um ponto real no Golfo da Guiné. latitude/longitude agora são nulas
até o sync preencher.
P3.9 — Seed falhava e saía com código 0
P3.10 — Validação duplicada no controller, já feita pelo handler global
Testes: +5 (60 -> 65).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Pmgii3S5sfMZghFqQXhbkH
P2.1 — Retake deixava diagnóstico órfão na fila O INSERT saiu de handleSaveToHistory, que o produtor acionava, para o fluxo automático pós-inferência. handleResetCamera limpava só o state do React, sem DELETE, então cada tentativa descartada subia para o servidor no sync seguinte e ainda consumia um PUT no S3 — num app de campo onde 3 a 5 tentativas até um bom enquadramento é o comportamento normal documentado. A lógica foi extraída para lib/diagnosticDraftService.ts em vez de ficar dentro do camera.tsx (1.969 linhas, sem cobertura de teste): assim é testável, e o arquivo para de crescer. Só descarta o que ainda está PENDING — linha sincronizada é histórico do produtor, não rascunho. O descarte aguarda a promessa de persistência em voo, porque o local_id pode ainda não ter chegado ao state quando o produtor toca em Retake. P3.4 — Falha do banco local se passava por indisponibilidade do LLM O try/catch envolvia o api.post E o UPDATE no SQLite. Uma falha local (ex.: FK de llm_doenca_id com catálogo desatualizado) virava LLM_UNAVAILABLE; o app marcava SKIPPED e o guard do servidor impedia a auto-correção no sync seguinte. Cliente e servidor divergiam permanentemente, com a segunda opinião real existindo no servidor e nunca aparecendo para o produtor. P3.5 — Migration v5 deixava diagnósticos legados como PENDING ADD COLUMN NOT NULL DEFAULT preenche o default em todas as linhas existentes. Elas são anteriores à cross-validation e nunca voltam ao fluxo, mas como o valor não é nulo o fallback `?? 'SKIPPED'` do syncService não dispara e elas chegam ao servidor como PENDING para sempre. Backfill na v5 (onde toda linha é comprovadamente legada) e migração v6 para quem já rodou a v5 defeituosa. Testes: +5 (49 -> 54). runMigrationsAndSeed exportada para permitir teste. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Pmgii3S5sfMZghFqQXhbkH
Mapeia cada achado ao commit que o corrigiu, registra as decisões que foram além da correção pontual e o bug latente (429 virando 500) descoberto ao ativar os rate limits. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Pmgii3S5sfMZghFqQXhbkH
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.
Corrige os 20 achados únicos levantados pelo ultrareview sobre o PR #1 e pelo
/code-review maxsobrebackend/src.Base é a branch do PR #1 para que o diff mostre só as correções. Merge desta PR primeiro, depois o PR #1 leva tudo para a
main.Resultado
Cada correção seguiu TDD: teste que falha primeiro, falha verificada pelo motivo certo, depois a correção mínima. O RED mais informativo foi o do rate limit —
expected '60' to be '10'em todas as rotas, provando o achado antes de tocar no código.Os quatro que bloqueavam teste de campo
P1.1 — a fila de sync travava para sempre.
SaudáveleFitotoxicidadesão gravados comdoenca_idnulo, mas o schema exigia uuid. O.parse()estourava no primeiro item inválido e devolvia 400 para o lote inteiro, derrubando até 49 diagnósticos válidos. Como o erro era anterior ao loop por item, eles nunca viravamFAILED: ficavamPENDINGe a fila nunca mais drenava.Saudávelé provavelmente o resultado mais comum em campo.P1.2 — nenhum rate limit por rota estava ativo. Os cinco eram passados como
confignoapp.register(), que é opção do plugin e não da rota. Tudo rodava no global de 60/min.P1.3 — o veredito de cross-validation era forjável. O INSERT gravava
crossValidationStatusellm_*direto do cliente, sem o guard que a branch de UPDATE tinha. Dava para POSTarCONFIRMEDcom texto arbitrário e o/diagnosis/cross-validatedevolvia aquilo como segunda opinião, sem carregar imagem nem chamar o LLM.P1.4 — carga de imagem do S3 sem teto. Combinado com P1.2, DoS trivial.
Decisões que foram além da correção pontual
doenca_ide coordenadas viraram nullable. Descartar os diagnósticos especiais jogaria fora dado real de campo, que é o que o RF05 existe para preservar; e(0,0)é um ponto real no Golfo da Guiné.llm_doenca_nome— sem ela a repetição idempotente devolvianullonde a primeira chamada devolvia o que o LLM afirmou, e a RF08 proíbe ocultar divergência.RATE_LIMIT_ENABLED, ligado por padrão.lib/diagnosticDraftService.tsextraído decamera.tsx, que tem 1.969 linhas e zero cobertura.Bug latente descoberto no caminho
O
errorResponseBuilderdo rate limit devolvia objeto semstatusCode, então exceder o limite virava 500 genérico em vez de 429. Ficou invisível enquanto os limites nunca disparavam.Atenção ao revisar
drizzle-kit push— não há migrations versionadas neste projeto. As três são aditivas ou de alargamento (NOT NULL→ nullable), seguras, mas exigem atenção em produção.userIdreal, já que a chave S3 agora precisa pertencer ao usuário autenticado. Os testes que quebraram foram fixture inválido, não regressão.Fora do escopo
O CI do frontend continua quebrado e não foi tocado:
react-native-vision-camera@5.0.11publica import sem extensão (./VisionCamera), inválido em ESM, e oexpo export --platform webcarrega isso pelo loader ESM do Node 22. Passa localmente no Node 26. Não estava entre os achados.Detalhamento completo, incluindo os refutados:
docs/reviews/2026-08-26-achados-baseline.md.🤖 Generated with Claude Code
https://claude.ai/code/session_01Pmgii3S5sfMZghFqQXhbkH