ADR — PR #1020

2026-05-18 10:00 — Triage CodeRabbit findings for PR #1020

Comment: Parallelize independent metric fetches
Priority: nice-to-have
Decision: accept
Rationale: Improving performance by running independent async operations in parallel is a good practice.
Comment: Use Pino logger instead of console.error
Priority: nice-to-have
Decision: accept
Rationale: Consistency with project’s structured logging guidelines.
Comment: Reuse DB client in getTemporalFailureCounts
Priority: nice-to-have
Decision: accept
Rationale: Better resource management by reusing the request-scoped database client.
Comment: Combine chained .where() calls using and()
Priority: must-fix
Decision: accept
Rationale: Chained .where() calls in Drizzle overwrite each other, leading to incorrect query results.
Comment: Fix undefined helpers in webapp pipeline API
Priority: must-fix
Decision: accept
Rationale: The endpoint currently references undefined functions, which will cause runtime errors.

Comment: Split auth check in internal pipeline handler to distinguish between 401 and 403
Priority: nice-to-have
Decision: accept
Rationale: Better API semantics for authentication vs authorization.
Comment: Wrap webapp pipeline response in { data: metrics } envelope
Priority: nice-to-have
Decision: accept
Rationale: Consistency with internal API route contract.
Comment: Add explicit sys-admin authorization check to webapp pipeline handler
Priority: must-fix
Decision: accept
Rationale: Security requirement to ensure only admins can access pipeline metrics.
Comment: Use explicit named exports for metrics helpers in packages/db/src/index.ts
Priority: nice-to-have
Decision: accept
Rationale: Better control over public API surface and tree-shaking support.
Comment: Use transaction for atomic count reads in calculateSuccessRate
Priority: must-fix
Decision: accept
Rationale: Ensures consistency of success rate metrics under concurrent writes.
Comment: Fix incorrect array indexing when reading count in getTemporalFailureCounts
Priority: must-fix
Decision: accept
Rationale: Current code incorrectly attempts to index an already-destructured result, causing undefined access.

Comment: Add requestId/correlationId to logger context
Priority: nice-to-have
Decision: accept
Rationale: Enhanced observability by including correlationId (webapp) and userEmail (internal) in error logs.
Comment: Fix lint warnings in packages/db
Priority: nice-to-have
Decision: accept
Rationale: Ensuring clean lint runs by prefixing unused mock parameters with underscore.
Comment: Verify all previously triaged findings
Priority: must-fix
Decision: accept
Rationale: Confirmed that all requested changes (parallelization, structured logging, client reuse, and and() operator) are correctly implemented and verified by type checking.