diff --git a/packages/deslop-js/src/utils/oxc-ast-node.ts b/packages/deslop-js/src/utils/oxc-ast-node.ts index 3eb7a1b8f..1cc1f6f1e 100644 --- a/packages/deslop-js/src/utils/oxc-ast-node.ts +++ b/packages/deslop-js/src/utils/oxc-ast-node.ts @@ -8,26 +8,11 @@ export interface OxcAstNode { export const isOxcAstNode = (value: unknown): value is OxcAstNode => Boolean(value) && typeof value === "object" && typeof (value as OxcAstNode).type === "string"; -export const getNodeStringField = (node: OxcAstNode, key: string): string | undefined => { +const getNodeStringField = (node: OxcAstNode, key: string): string | undefined => { const value = node[key]; return typeof value === "string" ? value : undefined; }; -export const getNodeChild = (node: OxcAstNode, key: string): OxcAstNode | undefined => { - const value = node[key]; - return isOxcAstNode(value) ? value : undefined; -}; - -export const getNodeChildArray = (node: OxcAstNode, key: string): OxcAstNode[] => { - const value = node[key]; - if (!Array.isArray(value)) return []; - const children: OxcAstNode[] = []; - for (const candidate of value) { - if (isOxcAstNode(candidate)) children.push(candidate); - } - return children; -}; - export const getIdentifierName = (node: unknown): string | undefined => { if (!isOxcAstNode(node)) return undefined; if (node.type !== "Identifier") return undefined; diff --git a/packages/react-doctor/src/cli/utils/build-run-event.ts b/packages/react-doctor/src/cli/utils/build-run-event.ts index 96049ad54..b3e8a7761 100644 --- a/packages/react-doctor/src/cli/utils/build-run-event.ts +++ b/packages/react-doctor/src/cli/utils/build-run-event.ts @@ -15,22 +15,14 @@ import { toSpanAttributes } from "./to-span-attributes.js"; import { withNamespace } from "./with-namespace.js"; import type { SentryRootSpan } from "./with-sentry-run-span.js"; -// A tag-like map: `null` denotes an absent signal and is dropped by -// `toSpanAttributes` so it never becomes a misleading `"null"` attribute. interface RunEventAttributes { [attributeName: string]: string | number | boolean | null; } -/** - * Outcome of one scan, attached to its root span (the canonical "wide event"). - * `result` is absent on the failure path (the scan threw before finalizing); - * `error` is present there so the event still records what happened. - */ export interface RunEventInput { readonly result?: InspectResult; - /** `"diff"` / `"full"` / `"staged"`. */ + /** `"full"` / `"diff"` / `"baseline"`. */ readonly mode: string; - /** Resolved scan scope: full | files | changed | lines. */ readonly scope: string; readonly parallel: boolean; readonly workerCount: number | undefined; @@ -40,42 +32,18 @@ export interface RunEventInput { readonly noScore: boolean; readonly respectInlineDisables: boolean; readonly showWarnings: boolean; - /** A custom `--output-dir` was passed for the full diagnostics dump. */ readonly usedOutputDir: boolean; readonly ignoredTagCount: number; readonly hasCustomConfig: boolean; readonly userConfig: ReactDoctorConfig | null; - // Lint / dead-code outcome — only known on the success path. The failure path - // (the scan threw) omits these rather than asserting a benign default. readonly didLintFail?: boolean; readonly lintFailureReasonKind?: string | null; readonly lintPartialFailureCount?: number; - // Total files the lint pass dropped (timeout-tripping batches that couldn't - // be split further). A per-scan outcome dim — the kill-metric signal for LPT: - // if cost-ordering front-loads the pathological bucket and drops MORE files, - // this rises on the `cost` cohort vs `arrival`. readonly lintDroppedFileCount?: number; readonly didDeadCodeFail?: boolean; - // `true` when the background supply-chain check hit its overlap budget and - // failed open to no diagnostics. The kill metric for the lint/supply-chain - // overlap watches the rate of this per supply-chain-eligible scan (filter to - // `scan.mode == "full"`, since a skipped check also reports `false`). Known on the - // success path and replayed from the cached payload on a cache hit — but a - // timed-out run is never cached (`shouldStoreScanPayload`), so a cache hit - // always reports `false`. Omitted only on the failure path (the scan threw - // before finalizing). readonly supplyChainOverlapTimedOut?: boolean; - /** - * Whether the dead-code pass ran concurrently with lint this scan (gate - * opened / overlap forced). Lets a query compare `runInspect` wall-clock - * grouped by overlap, and watch for an OOM/timeout regression on - * overlapped scans (the kill-metric for the overlap feature). - */ readonly deadCodeOverlapped?: boolean; - // A degraded baseline run (no delta computed) skips the CI gate, so the - // `wouldBlock` prediction must match — never block on its plain-diff findings. readonly gateExempt?: boolean; - /** Present only when the scan threw. */ readonly error?: unknown; } @@ -94,12 +62,6 @@ const resolveVersionPin = (versionInput: string | undefined): string | null => { return "pinned"; }; -// The blocking threshold for the `wouldBlock` signal. The action forwards -// its own `blocking` input (so we see the gate even though it's handled by -// the CLI exit code); otherwise fall back to the config value (new name, then -// the deprecated `failOn` alias), then the `"error"` default. A bare -// `--blocking` CLI flag (no action, no config) isn't visible here — an -// accepted gap, since CI gating runs through the action or config. const resolveTelemetryBlocking = (userConfig: ReactDoctorConfig | null): BlockingLevel => { const fromAction = process.env[ACTION_INPUT_ENVIRONMENT_VARIABLES.blocking]; if (fromAction !== undefined && isValidBlockingLevel(fromAction)) { @@ -109,39 +71,42 @@ const resolveTelemetryBlocking = (userConfig: ReactDoctorConfig | null): Blockin }; const buildOutcomeAttributes = (input: RunEventInput): RunEventAttributes => { - // Failure path: the scan threw before producing a result. if (input.result === undefined) { const error = input.error; - const known = isReactDoctorError(error); + if (isReactDoctorError(error)) { + return withNamespace("outcome", { + status: "error", + exitCode: 1, + knownError: true, + errorTag: error.reason._tag, + }); + } return withNamespace("outcome", { status: "error", exitCode: 1, - knownError: known, - errorTag: known ? error.reason._tag : error instanceof Error ? error.name : null, + knownError: false, + errorTag: error instanceof Error ? error.name : null, }); } const result = input.result; const summary = summarizeDiagnostics(result.diagnostics); const blockingLevel = resolveTelemetryBlocking(input.userConfig); - // Mirror the CLI's real blocking gate (cli/commands/inspect.ts → finalizeScans): - // it tests the threshold against diagnostics filtered for the `ciFailure` - // surface (weak-signal `design`-tagged rules are dropped by default), so the - // wide event's `outcome.wouldBlock`/`outcome.status`/`outcome.exitCode` can't disagree with the actual - // process exit. The descriptive totals below still reflect the full findings. const gateDiagnostics = filterDiagnosticsForSurface( result.diagnostics, "ciFailure", input.userConfig, ); - // `scoreOnly` runs never raise a non-zero exit (finalizeScans guards the gate - // on `!isScoreOnly`), and a degraded baseline run (`gateExempt`) skips the - // gate too — keep `outcome.wouldBlock`/`outcome.status`/`outcome.exitCode` consistent with the real exit. const wouldBlock = !input.scoreOnly && !input.gateExempt && shouldBlockCi(gateDiagnostics, blockingLevel); const hasSkippedChecks = result.skippedChecks.length > 0; const isClean = result.diagnostics.length === 0 && !hasSkippedChecks; - const outcome = wouldBlock ? "blocked" : isClean ? "clean" : "ok"; + let outcome = "ok"; + if (wouldBlock) { + outcome = "blocked"; + } else if (isClean) { + outcome = "clean"; + } const firings = summarizeRuleFirings(result.diagnostics); const countByRule = new Map(); @@ -162,18 +127,10 @@ const buildOutcomeAttributes = (input: RunEventInput): RunEventAttributes => { } } - // Widest-blast-radius rule: how many files the single most-spread rule - // touches (and its site count). Lets a query see how common migration-scale - // buckets are and calibrate MIGRATION_SCALE_RULE_FILE_COUNT — the threshold - // that fires the "sample before you sweep" advisory — against real scans. const largestRuleBucket = buildRuleBlastRadii(result.diagnostics)[0] ?? null; let diagnosticsInTestFiles = 0; let diagnosticsInStoryFiles = 0; - // Root-cause grouping rollup: how many distinct fix groups, and how many - // findings they cover. `fixGroupedFindings - fixGroups` is the number of - // findings that collapse away (one fix, not N tasks) — the signal that says - // whether this feature fires on real repos and how much it folds. const findingsPerFixGroup = new Map(); for (const diagnostic of result.diagnostics) { if (diagnostic.fileContext === "test") diagnosticsInTestFiles += 1; @@ -188,8 +145,6 @@ const buildOutcomeAttributes = (input: RunEventInput): RunEventAttributes => { let fixGroupedFindings = 0; for (const count of findingsPerFixGroup.values()) fixGroupedFindings += count; - // Per-category diagnostic counts, keyed so the `diag` namespace yields - // `diag.category.` once prefixed. const categoryRollup: RunEventAttributes = {}; for (const [category, count] of countByCategory) { categoryRollup[`category.${toCategoryKey(category)}`] = count; @@ -227,9 +182,6 @@ const buildOutcomeAttributes = (input: RunEventInput): RunEventAttributes => { failureReasonKind: input.lintFailureReasonKind ?? null, partialFailureCount: input.lintPartialFailureCount ?? null, droppedFileCount: input.lintDroppedFileCount ?? null, - // Per-file lint cache outcome. Numeric so Sentry can `p75(lint.cacheHitRatio)`; - // all `null` when the cache was off/bypassed so "no cache" reads distinctly - // from a 0% hit rate (`toSpanAttributes` drops the nulls). cacheHitFiles: result.lintCacheHitFileCount ?? null, cacheTotalFiles: result.lintCacheTotalFileCount ?? null, cacheHitRatio: @@ -254,12 +206,6 @@ const buildOutcomeAttributes = (input: RunEventInput): RunEventAttributes => { largestRuleBucketRule: largestRuleBucket ? largestRuleBucket.ruleKey : null, }), }; - // Baseline (PR-introduced-issues-only) signal. Emitted only for baseline runs - // so non-baseline scans stay clean: a computed run carries the delta (`new` is - // the introduced count == diag.total, plus `fixed` and base `baseTotal`) and - // `degraded: false`; a degraded run (base ref unfetchable or lint failed, - // surfaced via `gateExempt`) carries only `degraded: true`. The pair lets a - // query compute the degradation rate over all baseline runs. if (result.baselineDelta) { Object.assign( attributes, @@ -271,7 +217,7 @@ const buildOutcomeAttributes = (input: RunEventInput): RunEventAttributes => { }), ); } else if (input.gateExempt) { - attributes["baseline.degraded"] = true; + Object.assign(attributes, withNamespace("baseline", { degraded: true })); } return attributes; }; @@ -281,10 +227,6 @@ const buildActionAttributes = (): RunEventAttributes => { return withNamespace("action", { actorAssociation: githubActorAssociation ?? null, runnerOs: detectRunnerOs(), - // Action knobs: present only when the official action forwarded them, so - // they're `null` (dropped) for any non-action run. The action's `blocking` - // is already captured as `outcome.blocking` (resolveTelemetryBlocking - // prefers it). comment: readEnvBoolean(ACTION_INPUT_ENVIRONMENT_VARIABLES.comment), reviewComments: readEnvBoolean(ACTION_INPUT_ENVIRONMENT_VARIABLES.reviewComments), versionPin: resolveVersionPin(process.env[ACTION_INPUT_ENVIRONMENT_VARIABLES.version]), @@ -310,27 +252,10 @@ const buildScanAttributes = (input: RunEventInput): RunEventAttributes => { hasCustomConfig: input.hasCustomConfig, rulesConfigured: ruleKeys.length, rulesDisabled: ruleKeys.filter((key) => ruleOverrides[key] === "off").length, - // Scan extent — how many files this run covered (the denominator for - // `diag.affectedFiles`). Known only on the success path. fileCount: input.result?.scannedFileCount ?? null, }); }; -/** - * Projects a scan into the namespaced attribute set for its root span — the - * canonical per-scan "wide event". Every attribute carries a dotted namespace - * that groups it by concept (`scan.*` config, `action.*` CI knobs, `outcome.*` - * verdict, `diag.*` findings, `score.*`, `lint.*`, `deadCode.*`, - * `supplyChain.*`, `timing.*`, `migration.*`, `baseline.*`) so the attributes - * tree up in Sentry's attribute browser and stay filter-/group-/aggregate-able - * in the Spans dataset. Pure and exported so the projection (outcome - * precedence, rule/category rollups, CI knobs, config shape) is unit-testable - * without a live Sentry client. `null` values are dropped so absent signals - * never become misleading `"null"` attributes. The run + project base context - * (version, command, ci/provider, framework, …) is already on the span from - * `withSentryRunSpan` / `recordSentryProjectContext`, so this adds only what - * those don't carry. - */ export const buildRunEventAttributes = ( input: RunEventInput, ): Record => diff --git a/packages/react-doctor/src/cli/utils/count-dropped-lint-files.ts b/packages/react-doctor/src/cli/utils/count-dropped-lint-files.ts index 304c876f8..a4278a472 100644 --- a/packages/react-doctor/src/cli/utils/count-dropped-lint-files.ts +++ b/packages/react-doctor/src/cli/utils/count-dropped-lint-files.ts @@ -1,14 +1,3 @@ -// Total source files the lint pass dropped, summed from the partial-failure -// strings on `InspectResult.lintPartialFailures`. Each dropped-files event -// emits exactly one message of the shape `"N file(s) failed to lint and were -// skipped …"` (built in core's `spawn-batches.ts`); other partial-failure -// strings (e.g. the react-hooks-js plugin-drop note) don't match the prefix -// and contribute 0. The count is more sensitive than the message count -// (`lintPartialFailureCount`) for the LPT kill metric — a batch that strands -// the timeout-tripping bucket can drop many files in a single message — so it -// rides the wide event as `lint.droppedFileCount`. Parsing the anchored prefix -// keeps the signal contained to one CLI util instead of plumbing a structured -// count through the Linter → run-inspect → cache-payload chain. const DROPPED_FILES_MESSAGE_PATTERN = /^(\d+) file\(s\) failed to lint and were skipped/; export const countDroppedLintFiles = (lintPartialFailures: ReadonlyArray): number => diff --git a/packages/react-doctor/src/cli/utils/with-namespace.ts b/packages/react-doctor/src/cli/utils/with-namespace.ts index d6367ffda..f9147cf58 100644 --- a/packages/react-doctor/src/cli/utils/with-namespace.ts +++ b/packages/react-doctor/src/cli/utils/with-namespace.ts @@ -1,15 +1,3 @@ -/** - * Prefixes every key in a flat attribute group with `namespace.`, so a logical - * group (scan config, diagnostics rollup, score, …) carries one dotted - * namespace applied in a single place rather than hand-spelled into each key — - * which is how the wide event drifted into a flat, half-namespaced soup. The - * dotted prefix is what makes the attributes tree up in Sentry's attribute - * browser and stay group-/filter-/aggregate-able in the Spans dataset. - * - * Value types are preserved verbatim (numbers stay numbers so Sentry infers a - * numeric attribute and `p75(...)`/`avg(...)` work; `null` is kept so - * `toSpanAttributes` can drop absent signals rather than coerce them). - */ export const withNamespace = ( namespace: string, attributes: Record, diff --git a/packages/react-doctor/tests/with-namespace.test.ts b/packages/react-doctor/tests/with-namespace.test.ts new file mode 100644 index 000000000..cb521cb40 --- /dev/null +++ b/packages/react-doctor/tests/with-namespace.test.ts @@ -0,0 +1,22 @@ +import { describe, expect, it } from "vite-plus/test"; +import { withNamespace } from "../src/cli/utils/with-namespace.js"; + +describe("withNamespace", () => { + it("prefixes flat and dotted keys while preserving values", () => { + expect( + withNamespace("diag", { + total: 3, + "category.performance": 2, + clean: false, + topRule: "react-doctor/no-foo", + omitted: null, + }), + ).toEqual({ + "diag.total": 3, + "diag.category.performance": 2, + "diag.clean": false, + "diag.topRule": "react-doctor/no-foo", + "diag.omitted": null, + }); + }); +});