|
| 1 | +// altimate_change start — review feature telemetry. |
| 2 | +// |
| 3 | +// The review engine has two callers: the `review` CLI command and the `dbt_pr_review` tool. They |
| 4 | +// share this helper so there is one telemetry contract rather than two that drift — the zero-fill, |
| 5 | +// the privacy filtering and the failure classification all live here. |
| 6 | +// |
| 7 | +// Caller attribution needs no code: neither event declares a `source` field, so the envelope's |
| 8 | +// process-level `source` (from Flag.ALTIMATE_CLI_CLIENT) passes through untouched. A caller that |
| 9 | +// exports that variable is attributed automatically; one that does not reports `cli`. |
| 10 | +import { Telemetry } from "../telemetry" |
| 11 | +import { ReviewCategory, type Finding } from "./finding" |
| 12 | +import type { VerdictEnvelope } from "./verdict" |
| 13 | +import type { PostResult } from "./post-github" |
| 14 | + |
| 15 | +export type ReviewInvocation = "cli" | "tool" |
| 16 | + |
| 17 | +/** |
| 18 | + * Count surfaced findings by category, zero-filled across the whole enum. |
| 19 | + * |
| 20 | + * Zero-filled so a category that never fires is distinguishable from one that was never possible |
| 21 | + * in this run — an absent key and a zero mean different things to whoever reads the dashboard. |
| 22 | + * Keys come from `ReviewCategory.options`, never from the finding values themselves: |
| 23 | + * `Telemetry.aggregateFindings` accepts arbitrary strings and returns only observed keys, so a |
| 24 | + * malformed category would otherwise become a new dimension. |
| 25 | + */ |
| 26 | +function countByCategory(findings: Finding[]): Record<string, number> { |
| 27 | + // Prototype-less, and membership tested with Object.hasOwn: `{}` plus `in` accepted every |
| 28 | + // Object.prototype member, so a finding categorised `toString` both minted a dimension and |
| 29 | + // evaluated `<native function> + 1` into a Record<string, number>. Zod makes that unreachable |
| 30 | + // today, but this guard exists precisely for the case where validation was bypassed. |
| 31 | + const counts: Record<string, number> = Object.create(null) |
| 32 | + for (const category of ReviewCategory.options) counts[category] = 0 |
| 33 | + for (const finding of findings) { |
| 34 | + if (Object.hasOwn(counts, finding.category)) counts[finding.category] += 1 |
| 35 | + } |
| 36 | + return counts |
| 37 | +} |
| 38 | + |
| 39 | +/** |
| 40 | + * Classify a thrown review failure without threading typed errors through the engine. |
| 41 | + * |
| 42 | + * Only two failure modes actually propagate — everything else in the engine degrades rather than |
| 43 | + * throwing (missing manifests, dispatcher failures and the AI lane are all caught and turned into |
| 44 | + * empty or degraded results). So this deliberately recognises two and calls the rest `error` |
| 45 | + * rather than inventing buckets that can never occur. |
| 46 | + * |
| 47 | + * Matching is on the fixed prefix the config loader throws with, and on the spawn identity of the |
| 48 | + * git child process (`err.cmd`, set by `execFile`) — not broad substring matching over the |
| 49 | + * message, which would drift the moment anything is reworded. A `message.includes("git diff")` |
| 50 | + * fallback used to sit below the `cmd` check; it was unreachable for the real git path (execFile |
| 51 | + * always sets `cmd`, and its message begins "Command failed: ") and contradicted this paragraph. |
| 52 | + * |
| 53 | + * The `Failed to load` prefix is itself string matching. It is accurate against the config loader |
| 54 | + * today; a typed error at the throw site is what would make it robust. |
| 55 | + */ |
| 56 | +export function classifyReviewFailure(err: unknown): "config_error" | "git_error" | "error" { |
| 57 | + const message = err instanceof Error ? err.message : String(err) |
| 58 | + if (message.startsWith("Failed to load")) return "config_error" |
| 59 | + const cmd = (err as { cmd?: unknown } | undefined)?.cmd |
| 60 | + if (typeof cmd === "string" && /(^|[\\/\s])git(\s|$)/.test(cmd)) return "git_error" |
| 61 | + return "error" |
| 62 | +} |
| 63 | + |
| 64 | +/** |
| 65 | + * Map a PostResult onto the outcome enum. |
| 66 | + * |
| 67 | + * `PostResult` cannot express finer states than this: an inline fallback and a recorded post error |
| 68 | + * can coexist with a real review id, and `postError` is not cleared when the retry succeeds. So |
| 69 | + * everything short of a clean full post collapses to `partial` rather than pretending to a |
| 70 | + * precision the shape does not have. A throw before the summary is posted never reaches here — the |
| 71 | + * caller reports `summary_failed` for that. |
| 72 | + */ |
| 73 | +export function classifyPostOutcome(result: PostResult): "full" | "partial" { |
| 74 | + if (result.inlineFellBack || result.postError || result.reviewId === undefined) return "partial" |
| 75 | + return "full" |
| 76 | +} |
| 77 | + |
| 78 | +/** Emitted once per engine invocation, whichever caller reached it. */ |
| 79 | +export function emitReviewRun(input: { |
| 80 | + invocation: ReviewInvocation |
| 81 | + durationMs: number |
| 82 | + /** Empty on the CLI path, which has no chat session. */ |
| 83 | + sessionID: string |
| 84 | + envelope?: VerdictEnvelope |
| 85 | + error?: unknown |
| 86 | +}): void { |
| 87 | + try { |
| 88 | + const base = { |
| 89 | + type: "review_run" as const, |
| 90 | + timestamp: Date.now(), |
| 91 | + session_id: input.sessionID, |
| 92 | + invocation: input.invocation, |
| 93 | + duration_ms: input.durationMs, |
| 94 | + } |
| 95 | + |
| 96 | + if (!input.envelope) { |
| 97 | + Telemetry.track({ ...base, status: "failed", reason: classifyReviewFailure(input.error) }) |
| 98 | + return |
| 99 | + } |
| 100 | + |
| 101 | + const env = input.envelope |
| 102 | + Telemetry.track({ |
| 103 | + ...base, |
| 104 | + status: "completed", |
| 105 | + verdict: env.verdict, |
| 106 | + ideal_verdict: env.idealVerdict, |
| 107 | + // The effective mode, which config can set — not whatever the caller passed as a flag. |
| 108 | + mode: env.mode, |
| 109 | + tier: env.tier, |
| 110 | + // Optional in the schema and explicitly invalid as `false`, so normalise rather than copy. |
| 111 | + tier_forced: env.tierForced === true, |
| 112 | + degraded: env.summary.degraded, |
| 113 | + stale_manifest: env.staleManifest === true, |
| 114 | + critical: env.summary.critical, |
| 115 | + warning: env.summary.warning, |
| 116 | + suggestion: env.summary.suggestion, |
| 117 | + by_category: countByCategory(env.findings), |
| 118 | + }) |
| 119 | + } catch { |
| 120 | + // Telemetry must never fail a review. |
| 121 | + } |
| 122 | +} |
| 123 | + |
| 124 | +/** |
| 125 | + * Emitted on the CLI path only — the tool does not publish. |
| 126 | + * |
| 127 | + * CONTRACT: exactly one of these per *completed* review, never more and never fewer. A review that |
| 128 | + * threw never reached a publication phase, so it gets `review_run: failed` and no post event — |
| 129 | + * absence therefore means "the review failed", not "telemetry was lost". The caller enforces the |
| 130 | + * once-ness with a latch plus a `finally`; see cli/cmd/review.ts. |
| 131 | + */ |
| 132 | +export function emitReviewPostOutcome(input: { |
| 133 | + outcome: "not_requested" | "not_attempted" | "target_unresolved" | "full" | "partial" | "summary_failed" |
| 134 | + durationMs: number |
| 135 | + sessionID: string |
| 136 | +}): void { |
| 137 | + try { |
| 138 | + Telemetry.track({ |
| 139 | + type: "review_post_outcome", |
| 140 | + timestamp: Date.now(), |
| 141 | + session_id: input.sessionID, |
| 142 | + outcome: input.outcome, |
| 143 | + duration_ms: input.durationMs, |
| 144 | + }) |
| 145 | + } catch { |
| 146 | + // Telemetry must never fail a review. |
| 147 | + } |
| 148 | +} |
| 149 | +// altimate_change end |
0 commit comments