diff --git a/.changeset/review-out-of-lane.md b/.changeset/review-out-of-lane.md new file mode 100644 index 00000000..98ba79be --- /dev/null +++ b/.changeset/review-out-of-lane.md @@ -0,0 +1,5 @@ +--- +"review": minor +--- + +Out-of-lane observations are handed off, never dropped. Observed on the review-v1.4.0 re-run lifecycle: the skill-auditor found a real eventual-consistency bug (dedup reads via a Query immediately after PutMulti) and discarded it, verbatim "that's a correctness concern, not a quotable skill-rule violation, so I'll leave it"; correct under quote-the-rule, but the observation died and the defect shipped unflagged. The skill-auditor and every specialist lens may now return `out_of_lane_observations[]` (path, optional line, the concern stated concretely, a required concrete `failure_scenario`, optional `suggested_lane`) for real concerns their own mandate does not let them report. The orchestrator converts each one into a candidate comment with the code-assigned label `question (non-blocking)` (a handoff is not a vetted finding and can never block on its own; the claim-validator never upgrades severity) and routes it through the identical provenance gate, scope filter, claims.json, and validation path as every other candidate. The shape is validated by the new `validateOutOfLaneObservation` in `lib/finding-schema.ts` (a sibling type, so `FINDING_SCHEMA_VERSION` stays 2: no serialized finding is invalidated). diff --git a/workflows/review/README.md b/workflows/review/README.md index 573eac49..93794214 100644 --- a/workflows/review/README.md +++ b/workflows/review/README.md @@ -37,6 +37,11 @@ read-only **sub-agents** (it makes every GitHub and comment call itself): blocking first, with the resolved count, and an approval that resolved the last open threads states that every prior thread is resolved — resolving some threads never leaves the rest silently open. + A reviewer that surfaces a real concern its own mandate does not let it report — a + correctness problem the skill-auditor cannot quote a rule for, or something outside + a specialist lens's domain — hands it off as an `out_of_lane_observations[]` entry + instead of dropping it; the orchestrator routes each one into claim validation as a + non-blocking candidate (label code-assigned, so a handoff can never block on its own). 3. If those reviewers proposed any comments, **`claim-validator`** re-checks each one against the actual code (attacking the finding's stated failure scenario) and, for best-practice claims, against the relevant skill's diff --git a/workflows/review/eval/corpus/live.ts b/workflows/review/eval/corpus/live.ts index 9ca12eaf..890e4a9e 100644 --- a/workflows/review/eval/corpus/live.ts +++ b/workflows/review/eval/corpus/live.ts @@ -72,7 +72,19 @@ export type LiveDefectSpec = { */ export type CaseLive = { prContext: LivePrContext; - /** Case-dir-relative path to the post-change tree (default `tree`). */ + /** + * Case-dir-relative path to the post-change tree (default `tree`). + * + * Trees are deliberately minimal: validation requires only the changed + * files to exist. The staged copy of this tree is the sub-agent's whole + * world (its cwd, with no network), so the agent WILL often try to read + * an imported module or caller that is not there; that read returns an + * ordinary not-found tool error the agent tolerates and works around, + * not a run failure. The cost of a missing file is realism, not a crash: + * include the context files whose absence would change what a reviewer + * concludes (e.g. the decorator module a sibling handler imports), and + * nothing more. + */ tree: string; /** Labeled defects a live run must catch. */ mustCatchSpecs?: LiveDefectSpec[]; diff --git a/workflows/review/lib/finding-schema.test.ts b/workflows/review/lib/finding-schema.test.ts index be9c7a49..d0f771f8 100644 --- a/workflows/review/lib/finding-schema.test.ts +++ b/workflows/review/lib/finding-schema.test.ts @@ -10,6 +10,8 @@ import { validateFinding, isValidFinding, assertFinding, + validateOutOfLaneObservation, + isValidOutOfLaneObservation, } from "./finding-schema.ts"; /** @@ -384,3 +386,89 @@ describe("assertFinding", () => { } }); }); + +describe("validateOutOfLaneObservation", () => { + const makeValidObservation = ( + overrides: Record = {}, + ): Record => ({ + path: "services/foo/dedup.go", + line: 42, + observation: + "Dedup reads via an eventually-consistent Query immediately after PutMulti.", + failure_scenario: + "Two rapid submissions race the index; the second Query misses the first PutMulti and both rows persist.", + suggested_lane: "correctness", + ...overrides, + }); + + it("accepts a well-formed observation", () => { + const result = validateOutOfLaneObservation(makeValidObservation()); + expect(result.ok).toBe(true); + }); + + it("accepts the minimal shape (no line, no suggested_lane)", () => { + const minimal = makeValidObservation(); + delete minimal["line"]; + delete minimal["suggested_lane"]; + expect(validateOutOfLaneObservation(minimal).ok).toBe(true); + }); + + it("rejects a non-object", () => { + const result = validateOutOfLaneObservation("nope"); + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.errors).toEqual(["observation: must be an object"]); + } + }); + + it("requires path, observation, and failure_scenario", () => { + const result = validateOutOfLaneObservation({}); + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.errors).toContain("path: required non-empty string"); + expect(result.errors).toContain( + "observation: required non-empty string", + ); + expect(result.errors).toContain( + "failure_scenario: required non-empty string", + ); + } + }); + + it("collects every violation rather than failing on the first", () => { + const result = validateOutOfLaneObservation( + makeValidObservation({ + path: "", + line: 0, + observation: "", + suggested_lane: "", + }), + ); + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.errors.length).toBe(4); + } + }); + + it("rejects a non-integer or non-positive line when present", () => { + expect( + validateOutOfLaneObservation(makeValidObservation({line: 1.5})).ok, + ).toBe(false); + expect( + validateOutOfLaneObservation(makeValidObservation({line: -3})).ok, + ).toBe(false); + }); +}); + +describe("isValidOutOfLaneObservation", () => { + it("narrows well-formed input and rejects malformed input", () => { + expect( + isValidOutOfLaneObservation({ + path: "a.go", + observation: "A real concern.", + failure_scenario: "Input X produces wrong output Y.", + }), + ).toBe(true); + expect(isValidOutOfLaneObservation({path: "a.go"})).toBe(false); + }); +}); diff --git a/workflows/review/lib/finding-schema.ts b/workflows/review/lib/finding-schema.ts index fad19c98..112ea005 100644 --- a/workflows/review/lib/finding-schema.ts +++ b/workflows/review/lib/finding-schema.ts @@ -170,6 +170,46 @@ export type Finding = { model_authored_prose: string; }; +/** + * An out-of-lane observation: a real concern a reviewer surfaced that its own + * mandate does not let it report as a finding — e.g. the skill-auditor notices + * a correctness problem while checking a rule, but quote-the-rule rightly + * forbids reporting it as a skill violation. Production motivation: on the + * review-v1.4.0 re-run lifecycle the skill-auditor found a real + * eventual-consistency bug (dedup reads via a Query immediately after + * PutMulti) and dropped it — "that's a correctness concern, not a quotable + * skill-rule violation, so I'll leave it". Correct per quote-the-rule, but the + * observation died. + * + * The skill-auditor and every specialist lens may return these alongside + * `findings[]`; the orchestrator routes each one into claim validation as a + * non-blocking candidate (label code-assigned, never model-chosen), so a + * declined-as-out-of-lane observation is validated and surfaced rather than + * discarded. An observation is a handoff, not a vetted finding: it can never + * block on its own. + */ +export type OutOfLaneObservation = { + /** Repo-relative path the observation anchors on. */ + path: string; + /** RIGHT-side diff line, when the concern is line-specific. */ + line?: number; + /** One sentence: the concern, stated concretely. Model-authored. */ + observation: string; + /** + * One sentence: the concrete inputs/state and the wrong outcome they + * produce — the claim the claim-validator attacks, exactly as on a + * finding. An observation whose scenario cannot be stated concretely is + * not worth handing off. + */ + failure_scenario: string; + /** The lane the producer thinks owns this, e.g. `correctness`. Optional. */ + suggested_lane?: string; +}; + +export type ObservationValidationResult = + | {ok: true; observation: OutOfLaneObservation} + | {ok: false; errors: string[]}; + export type ValidationResult = | {ok: true; finding: Finding} | {ok: false; errors: string[]}; @@ -328,6 +368,60 @@ export const validateFinding = (input: unknown): ValidationResult => { return {ok: true, finding: input as Finding}; }; +/** + * Validate an untrusted value against the out-of-lane observation shape. + * Same error-collecting contract as {@link validateFinding}: every problem is + * returned, so a producer's malformed handoff is diagnosable from the run + * artifact rather than silently dropped. + */ +export const validateOutOfLaneObservation = ( + input: unknown, +): ObservationValidationResult => { + const errors: string[] = []; + + if (!isRecord(input)) { + return {ok: false, errors: ["observation: must be an object"]}; + } + + if (!isNonEmptyString(input["path"])) { + errors.push("path: required non-empty string"); + } + + const line = input["line"]; + if ( + line !== undefined && + (!Number.isInteger(line) || (line as number) < 1) + ) { + errors.push("line: must be a positive integer when present"); + } + + if (!isNonEmptyString(input["observation"])) { + errors.push("observation: required non-empty string"); + } + + if (!isNonEmptyString(input["failure_scenario"])) { + errors.push("failure_scenario: required non-empty string"); + } + + if ( + input["suggested_lane"] !== undefined && + !isNonEmptyString(input["suggested_lane"]) + ) { + errors.push("suggested_lane: must be a non-empty string when present"); + } + + if (errors.length > 0) { + return {ok: false, errors}; + } + + return {ok: true, observation: input as OutOfLaneObservation}; +}; + +/** Narrowing boolean wrapper around {@link validateOutOfLaneObservation}. */ +export const isValidOutOfLaneObservation = ( + input: unknown, +): input is OutOfLaneObservation => validateOutOfLaneObservation(input).ok; + /** Narrowing boolean wrapper around {@link validateFinding}. */ export const isValidFinding = (input: unknown): input is Finding => validateFinding(input).ok; diff --git a/workflows/review/review.md b/workflows/review/review.md index bbda645f..b96ae9b3 100644 --- a/workflows/review/review.md +++ b/workflows/review/review.md @@ -595,6 +595,22 @@ inline-comment path with no separate gate. Record each lens's `hunts[]` tri-stat (`ran` / `not-applicable` / `found`) alongside its findings in the lens's `out/.json` artifact (below); the hunts are provenance/metrics, not comments, so they are not posted. +**Route out-of-lane observations into the candidate set (code-owned label).** The +`skill-auditor` and every specialist lens may return `out_of_lane_observations[]` +alongside their findings: real concerns their own mandate does not let them report +(for the skill-auditor, a concern that is not a quotable skill-rule violation; for a +lens, a concern outside its domain). Do not discard these. Convert each observation +into a candidate comment in the same label-bearing shape as every other candidate: +`path`/`line` from the observation, `subject` from its `observation` text verbatim, +`failure_scenario` verbatim, and the label **`question (non-blocking)`** — the label +is code-assigned, never model-chosen: an out-of-lane observation is a handoff, not a +vetted finding, so it can never block on its own (and the `claim-validator` never +upgrades severity). Set the candidate's `source` to `" (out-of-lane)"`. From +here each one flows through the identical change-provenance gate → scope filter → +`claims.json` → validation → posting path as every other candidate — do not shortcut +one past validation, and do not drop one because its producer was unsure of its lane +(that uncertainty is exactly why it is handed to the validator). + Parse each sub-agent's JSON and keep only the compact result. As you parse each one, also write its raw JSON verbatim to `/tmp/gh-aw/review/out/.json` (create the `out/` directory if needed) — one file per dispatched sub-agent, named after it, @@ -1554,6 +1570,15 @@ spirit-of-the-doc inference, no extrapolating a written rule to a case it does n name. (The `claim-validator` re-checks skill claims against the skill file's real text, so an unquotable claim will not survive anyway.) +**Hand off, never drop, an out-of-lane observation.** When your audit surfaces a +real concern that is **not** a quotable skill-rule violation — e.g. a correctness or +data-integrity problem you noticed while checking a rule — do not force it into a +violation and do not discard it: record it in `out_of_lane_observations[]` with a +concrete `failure_scenario`. The orchestrator routes it to claim validation as a +non-blocking candidate, so declining to report it as a violation (correct under +quote-the-rule) no longer kills the observation. An observation whose failure +scenario you cannot state concretely is not worth handing off. + **Stay on the changed lines.** Anchor every violation on a line this PR adds or modifies, and only report a violation the *change* commits — never audit untouched code that merely appears in surrounding context, and never re-litigate pre-existing @@ -1578,11 +1603,19 @@ Return ONLY this JSON object (no prose, no code fence): "label": "issue (blocking, best-practice)|suggestion (non-blocking, best-practice)", "failure_scenario": "one sentence: the concrete consequence of the breach (what goes wrong, for whom)", "subject": "one line naming the skill area", "discussion": "the rule violated and the fix, quoting both", "suggestion": "optional fix code" + }], + "out_of_lane_observations": [{ + "path": "...", "line": 0, + "observation": "one sentence: the concern, stated concretely", + "failure_scenario": "one sentence: the concrete inputs/state and the wrong outcome they produce", + "suggested_lane": "correctness" }] } `line` is a RIGHT-side diff line. `failure_scenario` is required on every finding: the concrete consequence of the breach, stated specifically enough for the -claim-validator to attack. If no skill is relevant or no violations exist, +claim-validator to attack. `out_of_lane_observations` carries the hand-off rule +above (omit it or return `[]` when there is nothing to hand off; `line` and +`suggested_lane` are optional). If no skill is relevant or no violations exist, return {"findings": []}. ## agent: `pattern-triage` @@ -2274,6 +2307,15 @@ diff looks clean, so the `not-applicable`/`ran` record proves it was checked. deserialization sink without validation or parameterization. `found` on an unguarded sink. +**Hand off, never drop, an out-of-lane observation.** When your review surfaces a +real concern **outside this lens's domain** — noticed while tracing a caller or +reading surrounding context — do not force it into `findings[]` and do not discard +it: record it in `out_of_lane_observations[]` (below) with a concrete +`failure_scenario`. The orchestrator routes it to claim validation as a +non-blocking candidate, so staying in your lane no longer kills the observation. +Omit the field or return `[]` when there is nothing to hand off; `line` and +`suggested_lane` are optional. + ### Output Return ONLY this JSON object (no prose, no code fence). Every finding is a structured finding-schema object — do **not** emit a Conventional-Comment `label`; the orchestrator @@ -2293,6 +2335,7 @@ computes the label from `severity` + `lens` in code. "suggested_patch": "optional replacement/patch text", "pre_merge_obligation": "optional: a condition that must hold before merge" }], + "out_of_lane_observations": [{"path": "...", "line": 0, "observation": "one sentence: the concern, stated concretely", "failure_scenario": "one sentence: the concrete inputs/state and the wrong outcome they produce", "suggested_lane": "correctness"}], "hunts": [{"hunt": "authz-on-new-endpoint", "state": "ran|not-applicable|found"}] } Schema rules: `schema_version` is `2`; `lens` is exactly `security-auth`; `id` is unique @@ -2369,6 +2412,15 @@ emits a finding whose `producing_hunt` is the hunt name. - **`pii-to-model-or-logs`** — PII/sensitive fields sent to a model or written to a generation log unredacted. `found` on real exposure. +**Hand off, never drop, an out-of-lane observation.** When your review surfaces a +real concern **outside this lens's domain** — noticed while tracing a caller or +reading surrounding context — do not force it into `findings[]` and do not discard +it: record it in `out_of_lane_observations[]` (below) with a concrete +`failure_scenario`. The orchestrator routes it to claim validation as a +non-blocking candidate, so staying in your lane no longer kills the observation. +Omit the field or return `[]` when there is nothing to hand off; `line` and +`suggested_lane` are optional. + ### Output Return ONLY the finding-schema JSON object below — no Conventional-Comment `label` (the orchestrator computes it from `severity` + `lens`): @@ -2383,6 +2435,7 @@ orchestrator computes it from `severity` + `lens`): "model_authored_prose": "the comment the author will read", "suggested_patch": "optional", "pre_merge_obligation": "optional" }], + "out_of_lane_observations": [{"path": "...", "line": 0, "observation": "one sentence: the concern, stated concretely", "failure_scenario": "one sentence: the concrete inputs/state and the wrong outcome they produce", "suggested_lane": "correctness"}], "hunts": [{"hunt": "unmoderated-model-output", "state": "ran|not-applicable|found"}] } Schema rules are identical to every specialist lens: `schema_version` `2`; `lens` exactly @@ -2450,6 +2503,15 @@ finding whose `producing_hunt` is the hunt name. - **`unsubscribe-not-honored`** — a send that ignores opt-out / notification preferences. `found` when opt-out is bypassed. +**Hand off, never drop, an out-of-lane observation.** When your review surfaces a +real concern **outside this lens's domain** — noticed while tracing a caller or +reading surrounding context — do not force it into `findings[]` and do not discard +it: record it in `out_of_lane_observations[]` (below) with a concrete +`failure_scenario`. The orchestrator routes it to claim validation as a +non-blocking candidate, so staying in your lane no longer kills the observation. +Omit the field or return `[]` when there is nothing to hand off; `line` and +`suggested_lane` are optional. + ### Output Return ONLY the finding-schema JSON object below — no Conventional-Comment `label`: { @@ -2463,6 +2525,7 @@ Return ONLY the finding-schema JSON object below — no Conventional-Comment `la "model_authored_prose": "the comment the author will read", "suggested_patch": "optional", "pre_merge_obligation": "optional" }], + "out_of_lane_observations": [{"path": "...", "line": 0, "observation": "one sentence: the concern, stated concretely", "failure_scenario": "one sentence: the concrete inputs/state and the wrong outcome they produce", "suggested_lane": "correctness"}], "hunts": [{"hunt": "bulk-send-without-audience-filter", "state": "ran|not-applicable|found"}] } Schema rules are identical to every specialist lens (`lens` exactly `mass-comms-coppa`; @@ -2530,6 +2593,15 @@ finding whose `producing_hunt` is the hunt name. - **`unbounded-cache-or-collection`** — a cache/collection with no eviction, TTL, or size bound. `found` when growth is unbounded. +**Hand off, never drop, an out-of-lane observation.** When your review surfaces a +real concern **outside this lens's domain** — noticed while tracing a caller or +reading surrounding context — do not force it into `findings[]` and do not discard +it: record it in `out_of_lane_observations[]` (below) with a concrete +`failure_scenario`. The orchestrator routes it to claim validation as a +non-blocking candidate, so staying in your lane no longer kills the observation. +Omit the field or return `[]` when there is nothing to hand off; `line` and +`suggested_lane` are optional. + ### Output Return ONLY the finding-schema JSON object below — no Conventional-Comment `label`: { @@ -2543,6 +2615,7 @@ Return ONLY the finding-schema JSON object below — no Conventional-Comment `la "model_authored_prose": "the comment the author will read", "suggested_patch": "optional", "pre_merge_obligation": "optional" }], + "out_of_lane_observations": [{"path": "...", "line": 0, "observation": "one sentence: the concern, stated concretely", "failure_scenario": "one sentence: the concrete inputs/state and the wrong outcome they produce", "suggested_lane": "correctness"}], "hunts": [{"hunt": "cache-key-missing-identifier", "state": "ran|not-applicable|found"}] } Schema rules are identical to every specialist lens (`lens` exactly `caching-resource`; @@ -2610,6 +2683,15 @@ finding whose `producing_hunt` is the hunt name. - **`unbatched-backfill`** — a full-table `UPDATE`/backfill with no batching/chunking. `found` when the write is unbounded. +**Hand off, never drop, an out-of-lane observation.** When your review surfaces a +real concern **outside this lens's domain** — noticed while tracing a caller or +reading surrounding context — do not force it into `findings[]` and do not discard +it: record it in `out_of_lane_observations[]` (below) with a concrete +`failure_scenario`. The orchestrator routes it to claim validation as a +non-blocking candidate, so staying in your lane no longer kills the observation. +Omit the field or return `[]` when there is nothing to hand off; `line` and +`suggested_lane` are optional. + ### Output Return ONLY the finding-schema JSON object below — no Conventional-Comment `label`: { @@ -2623,6 +2705,7 @@ Return ONLY the finding-schema JSON object below — no Conventional-Comment `la "model_authored_prose": "the comment the author will read", "suggested_patch": "optional", "pre_merge_obligation": "optional" }], + "out_of_lane_observations": [{"path": "...", "line": 0, "observation": "one sentence: the concern, stated concretely", "failure_scenario": "one sentence: the concrete inputs/state and the wrong outcome they produce", "suggested_lane": "correctness"}], "hunts": [{"hunt": "non-nullable-column-without-default", "state": "ran|not-applicable|found"}] } Schema rules are identical to every specialist lens (`lens` exactly `data-migrations`; @@ -2689,6 +2772,15 @@ finding whose `producing_hunt` is the hunt name. - **`missing-idempotency-on-retryable-handler`** — a redeliverable handler doing a side-effecting op with no idempotency guard. `found` when redelivery double-applies. +**Hand off, never drop, an out-of-lane observation.** When your review surfaces a +real concern **outside this lens's domain** — noticed while tracing a caller or +reading surrounding context — do not force it into `findings[]` and do not discard +it: record it in `out_of_lane_observations[]` (below) with a concrete +`failure_scenario`. The orchestrator routes it to claim validation as a +non-blocking candidate, so staying in your lane no longer kills the observation. +Omit the field or return `[]` when there is nothing to hand off; `line` and +`suggested_lane` are optional. + ### Output Return ONLY the finding-schema JSON object below — no Conventional-Comment `label`: { @@ -2702,6 +2794,7 @@ Return ONLY the finding-schema JSON object below — no Conventional-Comment `la "model_authored_prose": "the comment the author will read", "suggested_patch": "optional", "pre_merge_obligation": "optional" }], + "out_of_lane_observations": [{"path": "...", "line": 0, "observation": "one sentence: the concern, stated concretely", "failure_scenario": "one sentence: the concrete inputs/state and the wrong outcome they produce", "suggested_lane": "correctness"}], "hunts": [{"hunt": "unawaited-async", "state": "ran|not-applicable|found"}] } Schema rules are identical to every specialist lens (`lens` exactly `concurrency-async`; @@ -2768,6 +2861,15 @@ finding whose `producing_hunt` is the hunt name. - **`federation-key-changed`** — a change to a federated key/reference/entity resolver that breaks composition. `found` when composition/resolution breaks. +**Hand off, never drop, an out-of-lane observation.** When your review surfaces a +real concern **outside this lens's domain** — noticed while tracing a caller or +reading surrounding context — do not force it into `findings[]` and do not discard +it: record it in `out_of_lane_observations[]` (below) with a concrete +`failure_scenario`. The orchestrator routes it to claim validation as a +non-blocking candidate, so staying in your lane no longer kills the observation. +Omit the field or return `[]` when there is nothing to hand off; `line` and +`suggested_lane` are optional. + ### Output Return ONLY the finding-schema JSON object below — no Conventional-Comment `label`: { @@ -2781,6 +2883,7 @@ Return ONLY the finding-schema JSON object below — no Conventional-Comment `la "model_authored_prose": "the comment the author will read", "suggested_patch": "optional", "pre_merge_obligation": "optional" }], + "out_of_lane_observations": [{"path": "...", "line": 0, "observation": "one sentence: the concern, stated concretely", "failure_scenario": "one sentence: the concrete inputs/state and the wrong outcome they produce", "suggested_lane": "correctness"}], "hunts": [{"hunt": "breaking-field-removal-or-retype", "state": "ran|not-applicable|found"}] } Schema rules are identical to every specialist lens (`lens` exactly @@ -2851,6 +2954,15 @@ finding whose `producing_hunt` is the hunt name. - **`format-switch-single-deploy`** — a writer switched to a new format/encoding/key set while old readers are still deployed. `found` on a single-phase switch. +**Hand off, never drop, an out-of-lane observation.** When your review surfaces a +real concern **outside this lens's domain** — noticed while tracing a caller or +reading surrounding context — do not force it into `findings[]` and do not discard +it: record it in `out_of_lane_observations[]` (below) with a concrete +`failure_scenario`. The orchestrator routes it to claim validation as a +non-blocking candidate, so staying in your lane no longer kills the observation. +Omit the field or return `[]` when there is nothing to hand off; `line` and +`suggested_lane` are optional. + ### Output Return ONLY the finding-schema JSON object below — no Conventional-Comment `label`: { @@ -2864,6 +2976,7 @@ Return ONLY the finding-schema JSON object below — no Conventional-Comment `la "model_authored_prose": "the comment the author will read", "suggested_patch": "optional", "pre_merge_obligation": "optional" }], + "out_of_lane_observations": [{"path": "...", "line": 0, "observation": "one sentence: the concern, stated concretely", "failure_scenario": "one sentence: the concrete inputs/state and the wrong outcome they produce", "suggested_lane": "correctness"}], "hunts": [{"hunt": "serialized-shape-change", "state": "ran|not-applicable|found"}] } Schema rules are identical to every specialist lens (`lens` exactly @@ -2932,6 +3045,15 @@ finding whose `producing_hunt` is the hunt name. - **`destructive-infra-change`** — an IaC change that destroys/replaces a stateful resource. `found` on an unguarded destructive change. +**Hand off, never drop, an out-of-lane observation.** When your review surfaces a +real concern **outside this lens's domain** — noticed while tracing a caller or +reading surrounding context — do not force it into `findings[]` and do not discard +it: record it in `out_of_lane_observations[]` (below) with a concrete +`failure_scenario`. The orchestrator routes it to claim validation as a +non-blocking candidate, so staying in your lane no longer kills the observation. +Omit the field or return `[]` when there is nothing to hand off; `line` and +`suggested_lane` are optional. + ### Output Return ONLY the finding-schema JSON object below — no Conventional-Comment `label`: { @@ -2945,6 +3067,7 @@ Return ONLY the finding-schema JSON object below — no Conventional-Comment `la "model_authored_prose": "the comment the author will read", "suggested_patch": "optional", "pre_merge_obligation": "optional" }], + "out_of_lane_observations": [{"path": "...", "line": 0, "observation": "one sentence: the concern, stated concretely", "failure_scenario": "one sentence: the concrete inputs/state and the wrong outcome they produce", "suggested_lane": "correctness"}], "hunts": [{"hunt": "flag-default-unsafe", "state": "ran|not-applicable|found"}] } Schema rules are identical to every specialist lens (`lens` exactly `deploy-infra-config`; @@ -3011,6 +3134,15 @@ finding whose `producing_hunt` is the hunt name. - **`currency-mismatch-or-missing`** — an amount handled without a currency, or arithmetic mixing currencies. `found` on a real mismatch. +**Hand off, never drop, an out-of-lane observation.** When your review surfaces a +real concern **outside this lens's domain** — noticed while tracing a caller or +reading surrounding context — do not force it into `findings[]` and do not discard +it: record it in `out_of_lane_observations[]` (below) with a concrete +`failure_scenario`. The orchestrator routes it to claim validation as a +non-blocking candidate, so staying in your lane no longer kills the observation. +Omit the field or return `[]` when there is nothing to hand off; `line` and +`suggested_lane` are optional. + ### Output Return ONLY the finding-schema JSON object below — no Conventional-Comment `label`: { @@ -3024,6 +3156,7 @@ Return ONLY the finding-schema JSON object below — no Conventional-Comment `la "model_authored_prose": "the comment the author will read", "suggested_patch": "optional", "pre_merge_obligation": "optional" }], + "out_of_lane_observations": [{"path": "...", "line": 0, "observation": "one sentence: the concern, stated concretely", "failure_scenario": "one sentence: the concrete inputs/state and the wrong outcome they produce", "suggested_lane": "correctness"}], "hunts": [{"hunt": "float-money", "state": "ran|not-applicable|found"}] } Schema rules are identical to every specialist lens (`lens` exactly `money-payments`; @@ -3093,6 +3226,15 @@ finding whose `producing_hunt` is the hunt name. - **`locale-unaware-formatting`** — a date/number/currency formatted without locale. `found` on locale-unaware formatting. +**Hand off, never drop, an out-of-lane observation.** When your review surfaces a +real concern **outside this lens's domain** — noticed while tracing a caller or +reading surrounding context — do not force it into `findings[]` and do not discard +it: record it in `out_of_lane_observations[]` (below) with a concrete +`failure_scenario`. The orchestrator routes it to claim validation as a +non-blocking candidate, so staying in your lane no longer kills the observation. +Omit the field or return `[]` when there is nothing to hand off; `line` and +`suggested_lane` are optional. + ### Output Return ONLY the finding-schema JSON object below — no Conventional-Comment `label`: { @@ -3106,6 +3248,7 @@ Return ONLY the finding-schema JSON object below — no Conventional-Comment `la "model_authored_prose": "the comment the author will read", "suggested_patch": "optional", "pre_merge_obligation": "optional" }], + "out_of_lane_observations": [{"path": "...", "line": 0, "observation": "one sentence: the concern, stated concretely", "failure_scenario": "one sentence: the concrete inputs/state and the wrong outcome they produce", "suggested_lane": "correctness"}], "hunts": [{"hunt": "hardcoded-user-facing-string", "state": "ran|not-applicable|found"}] } Schema rules are identical to every specialist lens (`lens` exactly `content-i18n`; unique