diff --git a/.changeset/review-finding-schema-foundations.md b/.changeset/review-finding-schema-foundations.md new file mode 100644 index 00000000..e30bc29d --- /dev/null +++ b/.changeset/review-finding-schema-foundations.md @@ -0,0 +1,5 @@ +--- +"review": minor +--- + +Add the versioned structured finding schema (`workflows/review/lib/finding-schema.ts`): every sub-agent finding now carries id, lens, anchor (line/range/file/pr-level), severity, confidence, evidence trace, optional suggested patch and pre-merge obligation, validated against an exported `FINDING_SCHEMA_VERSION`. Review submission is standardized on a single robust `submit-pull-request-review` call with a guaranteed non-empty body (the empty-body retry fallback is removed), and PR context (`pr-context.json`) is staged on disk once per run for all sub-agents, extending the existing diff staging. diff --git a/package.json b/package.json index 63681151..f81bb81a 100644 --- a/package.json +++ b/package.json @@ -23,11 +23,8 @@ "fast-glob": "^3.3.3", "memfs": "^4.51.0", "prettier": "^2.6.2", + "typescript": "^5.9.3", "vitest": "^4.0.10" }, - "packageManager": "pnpm@10.0.0+sha512.b8fef5494bd3fe4cbd4edabd0745df2ee5be3e4b0b8b08fa643aa3e4c6702ccc0f00d68fa8a8c9858a735a0032485a44990ed2810526c875e416f001b17df12b", - "dependencies": { - "@swc-node/register": "^1.11.1", - "typescript": "^5.9.3" - } + "packageManager": "pnpm@10.0.0+sha512.b8fef5494bd3fe4cbd4edabd0745df2ee5be3e4b0b8b08fa643aa3e4c6702ccc0f00d68fa8a8c9858a735a0032485a44990ed2810526c875e416f001b17df12b" } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 43946576..f679e932 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -7,13 +7,6 @@ settings: importers: .: - dependencies: - '@swc-node/register': - specifier: ^1.11.1 - version: 1.11.1(@swc/core@1.15.18)(@swc/types@0.1.25)(typescript@5.9.3) - typescript: - specifier: ^5.9.3 - version: 5.9.3 devDependencies: '@changesets/cli': specifier: ^2.29.8 @@ -21,6 +14,9 @@ importers: '@khanacademy/eslint-config': specifier: ^0.1.0 version: 0.1.0(eslint-config-prettier@8.5.0(eslint@8.15.0))(eslint-plugin-babel@5.3.1(eslint@8.15.0))(eslint-plugin-eslint-comments@3.2.0(eslint@8.15.0))(eslint-plugin-flowtype@5.10.0(eslint@8.15.0))(eslint-plugin-graphql@4.0.0(@types/node@25.3.3)(graphql@15.10.1)(typescript@5.9.3))(eslint-plugin-import@2.31.0(@typescript-eslint/parser@8.57.2(eslint@8.15.0)(typescript@5.9.3))(eslint@8.15.0))(eslint-plugin-jsx-a11y@6.10.2(eslint@8.15.0))(eslint-plugin-prettier@4.0.0(eslint-config-prettier@8.5.0(eslint@8.15.0))(eslint@8.15.0)(prettier@2.6.2))(eslint-plugin-react-hooks@4.6.2(eslint@8.15.0))(eslint-plugin-react-native-animation-linter@0.1.2(eslint@8.15.0))(eslint-plugin-react-native@3.11.0(eslint@8.15.0))(eslint-plugin-react@7.37.4(eslint@8.15.0))(eslint@8.15.0) + '@swc-node/register': + specifier: ^1.11.1 + version: 1.11.1(@swc/core@1.15.18)(@swc/types@0.1.25)(typescript@5.9.3) '@types/node': specifier: ^25.3.3 version: 25.3.3 @@ -54,6 +50,9 @@ importers: prettier: specifier: ^2.6.2 version: 2.6.2 + typescript: + specifier: ^5.9.3 + version: 5.9.3 vitest: specifier: ^4.0.10 version: 4.0.10(@types/node@25.3.3)(yaml@2.8.3) diff --git a/workflows/review/lib/finding-schema.test.ts b/workflows/review/lib/finding-schema.test.ts new file mode 100644 index 00000000..fbaac1cb --- /dev/null +++ b/workflows/review/lib/finding-schema.test.ts @@ -0,0 +1,374 @@ +import {describe, it, expect} from "vitest"; + +import { + FINDING_SCHEMA_VERSION, + KNOWN_LENSES, + SEVERITIES, + ANCHOR_TYPES, + MIN_CONFIDENCE, + MAX_CONFIDENCE, + validateFinding, + isValidFinding, + assertFinding, +} from "./finding-schema.ts"; + +/** + * Unit tests for the versioned structured finding schema/validator. + * Covers the exported version constant, well-formed findings across + * every anchor type + optional fields, and malformed findings for every + * required field — including the all-violations collection behavior the coder + * documented (so per-lens validator drop-rate stays diagnosable). + */ + +// A minimal well-formed finding. Individual tests clone + mutate this so a +// single field is the only thing under test. +const makeValidFinding = (overrides: Record = {}) => ({ + schema_version: FINDING_SCHEMA_VERSION, + id: "finding-1", + lens: "security-auth", + anchor: {type: "line", path: "src/app.ts", line: 42}, + severity: "blocking", + confidence: 0.9, + evidence_trace: ["src/app.ts:42 calls exec() with unsanitized input"], + producing_hunt: "security-auth/command-injection", + model_authored_prose: "User input flows unsanitized into a shell command.", + ...overrides, +}); + +describe("FINDING_SCHEMA_VERSION", () => { + it("is the exported monotonic constant (===1 at launch)", () => { + expect(FINDING_SCHEMA_VERSION).toBe(1); + expect(typeof FINDING_SCHEMA_VERSION).toBe("number"); + }); +}); + +describe("exported canonical lists", () => { + it("KNOWN_LENSES contains the eleven specialist lenses", () => { + for (const lens of [ + "security-auth", + "ai-safety-moderation", + "mass-comms-coppa", + "caching-resource", + "data-migrations", + "concurrency-async", + "api-federation-compat", + "cross-deploy-serialization", + "deploy-infra-config", + "money-payments", + "content-i18n", + ]) { + expect(KNOWN_LENSES).toContain(lens); + } + }); + + it("KNOWN_LENSES contains the always-on / triage reviewers", () => { + expect(KNOWN_LENSES).toContain("correctness"); + expect(KNOWN_LENSES).toContain("pattern-triage"); + expect(KNOWN_LENSES).toContain("first-principles"); + }); + + it("SEVERITIES is exactly blocking + advisory (#194 axis)", () => { + expect([...SEVERITIES]).toEqual(["blocking", "advisory"]); + }); + + it("ANCHOR_TYPES includes the required PR-level anchor", () => { + expect([...ANCHOR_TYPES]).toEqual(["line", "file", "pr"]); + }); + + it("confidence bounds are the closed unit interval", () => { + expect(MIN_CONFIDENCE).toBe(0); + expect(MAX_CONFIDENCE).toBe(1); + }); +}); + +describe("validateFinding — well-formed findings", () => { + it("accepts a minimal well-formed line-anchored finding", () => { + const result = validateFinding(makeValidFinding()); + expect(result.ok).toBe(true); + if (result.ok) { + expect(result.finding.id).toBe("finding-1"); + } + }); + + it("accepts a multi-line range anchor with an explicit side", () => { + const result = validateFinding( + makeValidFinding({ + anchor: { + type: "line", + path: "src/app.ts", + line: 50, + start_line: 42, + side: "RIGHT", + }, + }), + ); + expect(result.ok).toBe(true); + }); + + it("accepts a LEFT-side line anchor", () => { + const result = validateFinding( + makeValidFinding({ + anchor: { + type: "line", + path: "src/app.ts", + line: 7, + side: "LEFT", + }, + }), + ); + expect(result.ok).toBe(true); + }); + + it("accepts a file-level anchor", () => { + const result = validateFinding( + makeValidFinding({anchor: {type: "file", path: "src/app.ts"}}), + ); + expect(result.ok).toBe(true); + }); + + it("accepts a PR-level anchor with no path/line", () => { + const result = validateFinding( + makeValidFinding({anchor: {type: "pr"}}), + ); + expect(result.ok).toBe(true); + }); + + it("accepts advisory severity", () => { + expect( + validateFinding(makeValidFinding({severity: "advisory"})).ok, + ).toBe(true); + }); + + it("accepts confidence at both interval boundaries", () => { + expect(validateFinding(makeValidFinding({confidence: 0})).ok).toBe( + true, + ); + expect(validateFinding(makeValidFinding({confidence: 1})).ok).toBe( + true, + ); + }); + + it("accepts the optional suggested_patch + pre_merge_obligation when present", () => { + const result = validateFinding( + makeValidFinding({ + suggested_patch: "--- a/x\n+++ b/x\n@@ -1 +1 @@\n-a\n+b", + pre_merge_obligation: "Rotate the leaked key before merge.", + }), + ); + expect(result.ok).toBe(true); + }); + + it("accepts every KNOWN_LENSES value", () => { + for (const lens of KNOWN_LENSES) { + expect(validateFinding(makeValidFinding({lens})).ok).toBe(true); + } + }); +}); + +describe("validateFinding — malformed findings", () => { + const expectRejects = (input: unknown, matcher: RegExp) => { + const result = validateFinding(input); + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.errors.length).toBeGreaterThan(0); + expect(result.errors.some((e) => matcher.test(e))).toBe(true); + } + }; + + it("rejects a non-object input", () => { + expectRejects(null, /finding: must be an object/); + expectRejects("nope", /finding: must be an object/); + expectRejects([makeValidFinding()], /finding: must be an object/); + }); + + it("rejects an unrecognized schema_version (too low, too high, missing)", () => { + expectRejects(makeValidFinding({schema_version: 0}), /schema_version/); + expectRejects( + makeValidFinding({schema_version: FINDING_SCHEMA_VERSION + 1}), + /schema_version/, + ); + const noVersion: Record = {...makeValidFinding()}; + delete noVersion["schema_version"]; + expectRejects(noVersion, /schema_version/); + }); + + it("rejects a missing / empty id", () => { + expectRejects(makeValidFinding({id: ""}), /^id:/); + expectRejects(makeValidFinding({id: 123}), /^id:/); + }); + + it("rejects an unknown or non-string lens", () => { + expectRejects(makeValidFinding({lens: "no-such-lens"}), /^lens:/); + expectRejects(makeValidFinding({lens: 42}), /^lens:/); + }); + + it("rejects a bad severity", () => { + expectRejects(makeValidFinding({severity: "nit"}), /^severity:/); + }); + + it("rejects out-of-range / non-numeric confidence", () => { + expectRejects(makeValidFinding({confidence: -0.1}), /^confidence:/); + expectRejects(makeValidFinding({confidence: 1.1}), /^confidence:/); + expectRejects(makeValidFinding({confidence: NaN}), /^confidence:/); + expectRejects(makeValidFinding({confidence: "high"}), /^confidence:/); + }); + + it("rejects an empty / malformed evidence_trace", () => { + expectRejects(makeValidFinding({evidence_trace: []}), /evidence_trace/); + expectRejects( + makeValidFinding({evidence_trace: "not-array"}), + /evidence_trace/, + ); + expectRejects( + makeValidFinding({evidence_trace: [""]}), + /evidence_trace/, + ); + expectRejects( + makeValidFinding({evidence_trace: ["ok", 3]}), + /evidence_trace/, + ); + }); + + it("rejects a missing producing_hunt", () => { + expectRejects(makeValidFinding({producing_hunt: ""}), /producing_hunt/); + }); + + it("rejects a missing model_authored_prose", () => { + expectRejects( + makeValidFinding({model_authored_prose: ""}), + /model_authored_prose/, + ); + }); + + it("rejects present-but-empty optional fields", () => { + expectRejects( + makeValidFinding({suggested_patch: ""}), + /suggested_patch/, + ); + expectRejects( + makeValidFinding({pre_merge_obligation: ""}), + /pre_merge_obligation/, + ); + }); + + describe("anchor", () => { + it("rejects a non-object anchor", () => { + expectRejects( + makeValidFinding({anchor: "line"}), + /anchor: must be an object/, + ); + }); + + it("rejects an unknown anchor.type", () => { + expectRejects( + makeValidFinding({ + anchor: {type: "region", path: "x", line: 1}, + }), + /anchor\.type/, + ); + }); + + it("rejects a line/file anchor missing its path", () => { + expectRejects( + makeValidFinding({anchor: {type: "line", line: 1}}), + /anchor\.path/, + ); + expectRejects( + makeValidFinding({anchor: {type: "file"}}), + /anchor\.path/, + ); + }); + + it("rejects a non-positive / non-integer line", () => { + expectRejects( + makeValidFinding({anchor: {type: "line", path: "x", line: 0}}), + /anchor\.line/, + ); + expectRejects( + makeValidFinding({ + anchor: {type: "line", path: "x", line: 1.5}, + }), + /anchor\.line/, + ); + }); + + it("rejects a bad side", () => { + expectRejects( + makeValidFinding({ + anchor: {type: "line", path: "x", line: 1, side: "MIDDLE"}, + }), + /anchor\.side/, + ); + }); + + it("rejects an inverted range (start_line > line)", () => { + expectRejects( + makeValidFinding({ + anchor: {type: "line", path: "x", line: 5, start_line: 9}, + }), + /anchor\.start_line/, + ); + }); + + it("rejects a non-positive start_line", () => { + expectRejects( + makeValidFinding({ + anchor: {type: "line", path: "x", line: 5, start_line: 0}, + }), + /anchor\.start_line/, + ); + }); + }); + + it("collects ALL violations at once (per-lens drop-rate diagnosability)", () => { + const result = validateFinding({ + schema_version: 99, + id: "", + lens: "bogus", + anchor: {type: "line"}, + severity: "nit", + confidence: 5, + evidence_trace: [], + producing_hunt: "", + model_authored_prose: "", + }); + expect(result.ok).toBe(false); + if (!result.ok) { + // Every field above is wrong — expect a rich, multi-error report, + // not a fail-fast single message. + expect(result.errors.length).toBeGreaterThanOrEqual(8); + } + }); +}); + +describe("isValidFinding", () => { + it("narrows to true for a well-formed finding", () => { + expect(isValidFinding(makeValidFinding())).toBe(true); + }); + + it("returns false for a malformed finding", () => { + expect(isValidFinding({nope: true})).toBe(false); + expect(isValidFinding(makeValidFinding({lens: "bogus"}))).toBe(false); + }); +}); + +describe("assertFinding", () => { + it("returns the finding for well-formed input", () => { + const finding = assertFinding(makeValidFinding()); + expect(finding.id).toBe("finding-1"); + }); + + it("throws listing every violation for malformed input", () => { + expect(() => assertFinding({schema_version: 99})).toThrowError( + /Invalid finding/, + ); + try { + assertFinding(makeValidFinding({severity: "nit", confidence: 9})); + throw new Error("expected assertFinding to throw"); + } catch (err) { + const message = (err as Error).message; + expect(message).toMatch(/severity/); + expect(message).toMatch(/confidence/); + } + }); +}); diff --git a/workflows/review/lib/finding-schema.ts b/workflows/review/lib/finding-schema.ts new file mode 100644 index 00000000..c83da798 --- /dev/null +++ b/workflows/review/lib/finding-schema.ts @@ -0,0 +1,326 @@ +/** + * The versioned, structured finding schema shared by every reviewer + * sub-agent and the deterministic determinism-boundary code that consumes it + * (the computed verdict and the templated comment rendering). + * + * A "finding" is the single unit a lens sub-agent emits. Sub-agents write these + * as JSON (the #194 per-run sub-agent artifacts), so the wire keys are + * snake_case and this module validates that JSON before any downstream code + * (verdict, renderer, metrics) trusts it. The division of labor is fixed: + * + * - CODE owns structure: the schema version, labels/severity, anchors, + * templated wrapping. + * - MODELS own prose: only `model_authored_prose` (and the optional + * `suggested_patch` / `pre_merge_obligation` bodies) carry human-read text. + * + * Bumping the shape is a breaking change for artifacts on disk, so the version + * is an exported constant and every finding carries it; the validator rejects a + * finding stamped with a version it does not understand. + */ + +/** + * Monotonic schema version. Bump whenever a field is added/removed/retyped in a + * way that invalidates previously-serialized findings. Consumers compare the + * `schema_version` on each finding against this constant. + */ +export const FINDING_SCHEMA_VERSION = 1; + +/** + * The lenses (specialist + always-on) allowed to author a finding. The + * deterministic router dispatches to these; keeping the canonical list + * here means the validator can reject a finding attributed to an unknown lens + * (e.g. a typo or a decommissioned lens) rather than letting it flow downstream. + * + * The specialist lenses cover the path-gated risk areas; the + * remaining entries are the always-on / whole-change reviewers and triage. + */ +export const KNOWN_LENSES = [ + // Eleven specialist lenses. + "security-auth", + "ai-safety-moderation", + "mass-comms-coppa", + "caching-resource", + "data-migrations", + "concurrency-async", + "api-federation-compat", + "cross-deploy-serialization", + "deploy-infra-config", + "money-payments", + "content-i18n", + // Always-on / whole-change reviewers and triage. + "correctness", + "conventions", + "pattern-triage", + "first-principles", +] as const; + +export type Lens = typeof KNOWN_LENSES[number]; + +/** + * Per-finding severity. This is the blocking-relevant axis #194 introduced + * (blocking vs. advisory); the computed verdict turns the mix of + * severities plus posted-comment labels into a run-level outcome. Kept + * deliberately small — richer taxonomy lives in Conventional-Comment labels, + * which are code-owned at render time, not here. + */ +export const SEVERITIES = ["blocking", "advisory"] as const; + +export type Severity = typeof SEVERITIES[number]; + +/** + * Confidence axis (enables the eval suite's calibration metric). Numeric so a + * calibration curve can be plotted; constrained to the closed unit interval. + */ +export const MIN_CONFIDENCE = 0; +export const MAX_CONFIDENCE = 1; + +/** + * Where a finding is anchored. A finding may be: + * - `line`: a specific line (or line range) on one side of the diff — the + * common case, rendered as an inline review comment. + * - `file`: a whole file, when the concern is not line-specific. + * - `pr`: the PR as a whole (e.g. an architectural or cross-file concern) — + * the PR-level anchor type the schema is required to support. It carries no + * path/line and renders as a top-level review comment. + */ +export const ANCHOR_TYPES = ["line", "file", "pr"] as const; + +export type AnchorType = typeof ANCHOR_TYPES[number]; + +export type Side = "LEFT" | "RIGHT"; + +export type LineAnchor = { + type: "line"; + path: string; + /** 1-based line number the comment attaches to (the end line of a range). */ + line: number; + /** Diff side; defaults to the added ("RIGHT") side when omitted. */ + side?: Side; + /** 1-based first line of a multi-line range; when set, must be <= `line`. */ + start_line?: number; +}; + +export type FileAnchor = { + type: "file"; + path: string; +}; + +export type PrAnchor = { + type: "pr"; +}; + +export type Anchor = LineAnchor | FileAnchor | PrAnchor; + +/** + * The structured finding. `snake_case` keys mirror the on-disk JSON artifact + * that sub-agents emit. + */ +export type Finding = { + /** Schema version this finding was authored against. */ + schema_version: number; + /** Stable identifier, unique within a run (dedup + thumbs correlation). */ + id: string; + /** Which lens authored the finding. */ + lens: Lens; + /** Where the finding is anchored (line / file / PR-level). */ + anchor: Anchor; + /** Blocking-relevant severity. */ + severity: Severity; + /** Calibration confidence in [0, 1]. */ + confidence: number; + /** + * Ordered evidence the lens gathered to justify the finding (file/line + * references, tool observations, reasoning steps). At least one entry — a + * finding with no evidence is not actionable and is rejected. + */ + evidence_trace: string[]; + /** Optional unified-diff patch the author suggests (rendered as a suggestion). */ + suggested_patch?: string; + /** + * Optional pre-merge obligation text. Drives the conditional-approval + * (APPROVE-with-obligations) rendering. + */ + pre_merge_obligation?: string; + /** + * Identifier of the concrete hunt/sub-agent run that produced this finding + * (provenance for the live counters and validator drop-rate per lens). + */ + producing_hunt: string; + /** The single human-read sentence(s) authored by the model. */ + model_authored_prose: string; +}; + +export type ValidationResult = + | {ok: true; finding: Finding} + | {ok: false; errors: string[]}; + +const isRecord = (value: unknown): value is Record => + typeof value === "object" && value !== null && !Array.isArray(value); + +const isNonEmptyString = (value: unknown): value is string => + typeof value === "string" && value.length > 0; + +const validateAnchor = (value: unknown, errors: string[]): void => { + if (!isRecord(value)) { + errors.push("anchor: must be an object"); + return; + } + + const type = value["type"]; + if (!isNonEmptyString(type) || !ANCHOR_TYPES.includes(type as AnchorType)) { + errors.push(`anchor.type: must be one of ${ANCHOR_TYPES.join(", ")}`); + return; + } + + if (type === "pr") { + // PR-level anchor carries no path/line. + return; + } + + if (!isNonEmptyString(value["path"])) { + errors.push( + `anchor.path: required non-empty string for ${type} anchor`, + ); + } + + if (type === "line") { + const line = value["line"]; + if (!Number.isInteger(line) || (line as number) < 1) { + errors.push("anchor.line: must be a positive integer"); + } + + const side = value["side"]; + if (side !== undefined && side !== "LEFT" && side !== "RIGHT") { + errors.push('anchor.side: must be "LEFT" or "RIGHT" when present'); + } + + const startLine = value["start_line"]; + if (startLine !== undefined) { + if (!Number.isInteger(startLine) || (startLine as number) < 1) { + errors.push("anchor.start_line: must be a positive integer"); + } else if ( + Number.isInteger(line) && + (startLine as number) > (line as number) + ) { + errors.push("anchor.start_line: must be <= anchor.line"); + } + } + } +}; + +/** + * Validate an untrusted value (typically parsed sub-agent JSON) against the + * finding schema. Returns every problem found — callers log the full list so a + * lens's validator drop-rate is diagnosable — rather than failing on the first. + */ +export const validateFinding = (input: unknown): ValidationResult => { + const errors: string[] = []; + + if (!isRecord(input)) { + return {ok: false, errors: ["finding: must be an object"]}; + } + + const schemaVersion = input["schema_version"]; + if (schemaVersion !== FINDING_SCHEMA_VERSION) { + errors.push( + `schema_version: must equal ${FINDING_SCHEMA_VERSION} (got ${JSON.stringify( + schemaVersion, + )})`, + ); + } + + if (!isNonEmptyString(input["id"])) { + errors.push("id: required non-empty string"); + } + + if ( + !isNonEmptyString(input["lens"]) || + !KNOWN_LENSES.includes(input["lens"] as Lens) + ) { + errors.push(`lens: must be one of ${KNOWN_LENSES.join(", ")}`); + } + + validateAnchor(input["anchor"], errors); + + if ( + !isNonEmptyString(input["severity"]) || + !SEVERITIES.includes(input["severity"] as Severity) + ) { + errors.push(`severity: must be one of ${SEVERITIES.join(", ")}`); + } + + const confidence = input["confidence"]; + if ( + typeof confidence !== "number" || + Number.isNaN(confidence) || + confidence < MIN_CONFIDENCE || + confidence > MAX_CONFIDENCE + ) { + errors.push( + `confidence: must be a number in [${MIN_CONFIDENCE}, ${MAX_CONFIDENCE}]`, + ); + } + + const evidenceTrace = input["evidence_trace"]; + if ( + !Array.isArray(evidenceTrace) || + evidenceTrace.length === 0 || + !evidenceTrace.every(isNonEmptyString) + ) { + errors.push( + "evidence_trace: must be a non-empty array of non-empty strings", + ); + } + + if (!isNonEmptyString(input["producing_hunt"])) { + errors.push("producing_hunt: required non-empty string"); + } + + if (!isNonEmptyString(input["model_authored_prose"])) { + errors.push("model_authored_prose: required non-empty string"); + } + + // Optional fields: only constrained when present. + if ( + input["suggested_patch"] !== undefined && + !isNonEmptyString(input["suggested_patch"]) + ) { + errors.push("suggested_patch: must be a non-empty string when present"); + } + + if ( + input["pre_merge_obligation"] !== undefined && + !isNonEmptyString(input["pre_merge_obligation"]) + ) { + errors.push( + "pre_merge_obligation: must be a non-empty string when present", + ); + } + + if (errors.length > 0) { + return {ok: false, errors}; + } + + return {ok: true, finding: input as Finding}; +}; + +/** Narrowing boolean wrapper around {@link validateFinding}. */ +export const isValidFinding = (input: unknown): input is Finding => + validateFinding(input).ok; + +/** + * Throwing wrapper around {@link validateFinding} for call sites that treat a + * malformed finding as a programmer error. The thrown message lists every + * violation. + */ +export const assertFinding = (input: unknown): Finding => { + const result = validateFinding(input); + if (!result.ok) { + throw new Error( + `Invalid finding:\n${result.errors + .map((e) => ` - ${e}`) + .join("\n")}`, + ); + } + return result.finding; +}; diff --git a/workflows/review/review.md b/workflows/review/review.md index c2790010..f6e131f2 100644 --- a/workflows/review/review.md +++ b/workflows/review/review.md @@ -138,6 +138,26 @@ engine: model: claude-opus-4-8 timeout-minutes: 20 +# The shared review workflow is more than this markdown file: its deterministic +# pieces (the finding schema and validator today; the router, computed verdict, and +# comment renderer as they land) are TypeScript under `workflows/review/lib/` in +# Khan/actions. gh-aw's `source:` import copies only this .md file into a consuming +# repo, so the job fetches the code itself: check out Khan/actions at the pinned +# release below. The `ref` is the single version +# surface for prompt + code: it names the Khan/actions release this file ships in +# (changesets tag, `review-v`), and any release that changes the prompt or +# the lib bumps it. Steps that run lib scripts invoke them from `gh-aw-review-lib/` +# via `npx -y tsx