From 84cfc11e7443f42a2f3339acc9c18bb73e8e1297 Mon Sep 17 00:00:00 2001 From: amirbena Date: Thu, 24 Sep 2026 16:05:56 +0300 Subject: [PATCH 1/2] Add compatible structured output to GitHub PR review (#70) Co-Authored-By: Claude Sonnet 5 --- capabilities/reviewer-assist/capability.yaml | 6 +- docs/review-result/README.md | 6 +- docs/review-result/review-result-model.md | 13 +-- .../packaging/generate_package_manifest.py | 1 + scripts/packaging/package-manifest.json | 1 + skills/github-pr-review/README.md | 5 ++ skills/github-pr-review/SKILL.md | 5 ++ skills/github-pr-review/metadata/skill.yaml | 1 + .../policies/github-review.md | 5 ++ .../policies/structured-output.md | 89 +++++++++++++++++++ .../findings/test_structured_output_docs.py | 49 ++++++++++ 11 files changed, 173 insertions(+), 8 deletions(-) create mode 100644 skills/github-pr-review/policies/structured-output.md create mode 100644 tests/policy/review/findings/test_structured_output_docs.py diff --git a/capabilities/reviewer-assist/capability.yaml b/capabilities/reviewer-assist/capability.yaml index c9680f08..0fa1bff1 100644 --- a/capabilities/reviewer-assist/capability.yaml +++ b/capabilities/reviewer-assist/capability.yaml @@ -1,13 +1,17 @@ capability: reviewer-assist summary: >- The private Reviewer Brief: what changed, stated focus, manual-review - focus, and open questions, composed from the finalized result only. + focus, and open questions, composed from the finalized result only; + plus the optional machine-readable review result, a projection of the + same finalized result returned to the caller. loads: on-activation activation: - always, once findings, severity, coverage, and the verdict are final + - the caller explicitly requests a machine-readable (structured/JSON) result adapters: [github] files: - skills/github-pr-review/policies/reviewer-brief.md + - skills/github-pr-review/policies/structured-output.md - skills/github-pr-review/templates/reviewer-brief.md requires: [review-kernel] never: diff --git a/docs/review-result/README.md b/docs/review-result/README.md index 8bb0fce5..6d69d32a 100644 --- a/docs/review-result/README.md +++ b/docs/review-result/README.md @@ -7,8 +7,10 @@ Schema and example for the machine-readable form of one review's output. Like [`../review-telemetry/README.md`](../review-telemetry/README.md) and [`../finding-confidence/README.md`](../finding-confidence/README.md), these are repository-development docs: **not** packaged into either Skill -archive, and no packaged Skill resource depends on them. Nothing emits a -review result yet. +archive, and no packaged Skill resource depends on them. `github-pr-review` can emit a +review result on explicit request ([#70](https://github.com/amirbena/code-review-skill/issues/70), +[`structured-output.md`](../../skills/github-pr-review/policies/structured-output.md)); +`local-code-review` emission is [#69](https://github.com/amirbena/code-review-skill/issues/69). ## Document map diff --git a/docs/review-result/review-result-model.md b/docs/review-result/review-result-model.md index 9a0bcfee..868547b4 100644 --- a/docs/review-result/review-result-model.md +++ b/docs/review-result/review-result-model.md @@ -14,8 +14,8 @@ this record is the bug. Not packaged: no packaged Skill resource depends on this record, the schema, or the example (see [`../../AGENTS.md`](../../AGENTS.md), "Packaged -Skills are independent of repository-level instructions"). Nothing emits a -review result yet — see section 7. +Skills are independent of repository-level instructions"). `github-pr-review` emits +one on explicit request; local emission is pending — see section 7. ## 1. Files @@ -123,8 +123,11 @@ schema failing. | Consumers of the result | [#71](https://github.com/amirbena/code-review-skill/issues/71) | | Parent capability | [#44](https://github.com/amirbena/code-review-skill/issues/44) | -Until emission lands, this record, the schema, and the example are -contract-only: the packaged Skills still produce their existing Markdown -output, and [`finding.md`](../../shared/templates/finding.md)'s note that a +`github-pr-review` emits the result on explicit request, returned to the +caller only, with PR-specific field population owned by its +[`structured-output.md`](../../skills/github-pr-review/policies/structured-output.md) +([#70](https://github.com/amirbena/code-review-skill/issues/70)). Until +[#69](https://github.com/amirbena/code-review-skill/issues/69) lands, +`local-code-review` still produces only its existing Markdown output, and [`finding.md`](../../shared/templates/finding.md)'s note that a machine-readable renderer would be "another projection of the same fields" is what this schema is the first instance of. diff --git a/scripts/packaging/generate_package_manifest.py b/scripts/packaging/generate_package_manifest.py index 98b3e104..42703974 100644 --- a/scripts/packaging/generate_package_manifest.py +++ b/scripts/packaging/generate_package_manifest.py @@ -160,6 +160,7 @@ class Entry(NamedTuple): Entry("skills/github-pr-review/policies/finding-placement.md", "finding-placement-derivation"), Entry("skills/github-pr-review/policies/review-output.md", "publication-github"), Entry("skills/github-pr-review/policies/reviewer-brief.md", "reviewer-assist"), + Entry("skills/github-pr-review/policies/structured-output.md", "reviewer-assist"), Entry("skills/github-pr-review/policies/review-status-enforcement.md", "publication-github"), Entry("skills/github-pr-review/runbooks/passive-pr-review.md", None), Entry("skills/github-pr-review/runbooks/active-pr-review.md", None), diff --git a/scripts/packaging/package-manifest.json b/scripts/packaging/package-manifest.json index 085766a6..a7471ae3 100644 --- a/scripts/packaging/package-manifest.json +++ b/scripts/packaging/package-manifest.json @@ -81,6 +81,7 @@ { "source": "skills/github-pr-review/policies/finding-placement.md", "destination": "policies/finding-placement.md" }, { "source": "skills/github-pr-review/policies/review-output.md", "destination": "policies/review-output.md" }, { "source": "skills/github-pr-review/policies/reviewer-brief.md", "destination": "policies/reviewer-brief.md" }, + { "source": "skills/github-pr-review/policies/structured-output.md", "destination": "policies/structured-output.md" }, { "source": "skills/github-pr-review/policies/review-status-enforcement.md", "destination": "policies/review-status-enforcement.md" }, { "source": "skills/github-pr-review/runbooks/passive-pr-review.md", "destination": "runbooks/passive-pr-review.md" }, { "source": "skills/github-pr-review/runbooks/active-pr-review.md", "destination": "runbooks/active-pr-review.md" }, diff --git a/skills/github-pr-review/README.md b/skills/github-pr-review/README.md index b5f6abac..8e689963 100644 --- a/skills/github-pr-review/README.md +++ b/skills/github-pr-review/README.md @@ -83,6 +83,11 @@ Every result also carries a private, caller-only your stated focus, and 2-4 areas worth a manual look — that is structurally excluded from GitHub publication. +On explicit request it can also return a machine-readable review result +(same schema as local review; reviewed head SHA and decision populated) — +returned to you only, never published, and not the commit status. See +[`structured-output.md`](policies/structured-output.md). + ## Boundaries worth knowing | Rule | Short version | diff --git a/skills/github-pr-review/SKILL.md b/skills/github-pr-review/SKILL.md index 727a3a0c..cec654e8 100644 --- a/skills/github-pr-review/SKILL.md +++ b/skills/github-pr-review/SKILL.md @@ -247,6 +247,11 @@ If unavailable, do not fake success — fall back to passive review. composed once findings/severity/coverage/verdict are finalized and structurally excluded from anything published to GitHub — see [`policies/reviewer-brief.md`](policies/reviewer-brief.md). +- **On explicit request**, additionally a machine-readable review result + in the same schema as `local-code-review` (PR head SHA and decision + populated), returned to the caller only — never published, and not the + commit status — see + [`policies/structured-output.md`](policies/structured-output.md). - **Passive:** a human-readable report using the shared shape ([`review-summary.md`](../../shared/templates/review-summary.md)), returned to the caller, not published. diff --git a/skills/github-pr-review/metadata/skill.yaml b/skills/github-pr-review/metadata/skill.yaml index a83e3297..b1f83b2a 100644 --- a/skills/github-pr-review/metadata/skill.yaml +++ b/skills/github-pr-review/metadata/skill.yaml @@ -30,6 +30,7 @@ capabilities: repository_access_mode: api-only|optional-repository-backed|required-repository-backed # required checkout failure is REVIEW INCOMPLETE; see ../policies/repository-checkout.md parallel_review: optional # split review across workers per dimension when the runtime exposes a reliable capability; execution optimisation only; see ../../../shared/policies/parallel-review.md and ../policies/parallel-review.md review_action_authorization: active-mode-request-is-its-own-authorization # single canonical publication mode (PASSIVE|SEMI|ACTIVE); an explicit ACTIVE request is itself sufficient authorization to publish, subject to self-review/reviewer-independence/GitHub-permission/HEAD; ambiguity fails closed; see ../policies/review-action-authorization.md + structured_review_output: optional # explicit-request only; one schema-valid machine-readable review result (same schema as local review; reviewed head SHA, base/merge-base, derived decision populated) returned to the caller in every mode; never published to GitHub and never the commit status; changes no finding/severity/verdict; see ../policies/structured-output.md machine_readable_review_status: optional # one stable aggregated exact-HEAD GitHub status/check for the reviewed SHA, separate from native APPROVE/REQUEST_CHANGES; a blocking status is blocking-only enforcement (allowed for a self-review); a success status needs the same ACTIVE publication mode + reviewer independence as APPROVE and is never published by a self-review; a new HEAD inherits no green; see ../policies/review-status-enforcement.md required_check_setup: explicit-opt-in-only # adding the status context to base-branch required checks is a separate explicitly-requested action, minimal and preserving (never touches approval-count / dismiss_stale_reviews_on_push / require_last_push_approval / bypass actors / unrelated rules); never performed during an ordinary review; see ../policies/review-status-enforcement.md diff --git a/skills/github-pr-review/policies/github-review.md b/skills/github-pr-review/policies/github-review.md index 1a93bddb..36676e66 100644 --- a/skills/github-pr-review/policies/github-review.md +++ b/skills/github-pr-review/policies/github-review.md @@ -69,6 +69,11 @@ reviewer-brief.md private caller-facing Reviewer Brief; never published; composed only from the finalized analysis result above ↓ +structured-output.md optional machine-readable review result (same + schema as local review); PR head SHA + decision + populated; caller-only, never published; not the + commit status below + ↓ review-status-enforcement.md optional exact-HEAD machine-readable status; blocking vs. positive authority; enforcement detection; explicit opt-in required-check setup diff --git a/skills/github-pr-review/policies/structured-output.md b/skills/github-pr-review/policies/structured-output.md new file mode 100644 index 00000000..75dee690 --- /dev/null +++ b/skills/github-pr-review/policies/structured-output.md @@ -0,0 +1,89 @@ +# Policy — Structured Output + +Canonical semantics for the optional machine-readable review result +`github-pr-review` can return to its caller. Canonical index: +[`github-review.md`](github-review.md), which places this file after +[`reviewer-brief.md`](reviewer-brief.md): the document is a projection of +the same finalized analysis result and is composed only once findings, +severity, coverage, and the verdict are final. + +The document is the same review-result schema `local-code-review` emits +(this repository's `docs/review-result/`, a repository-development record +that is not packaged): identical field names, enums, and meaning. This +policy states only what is specific to a pull request — which values +populate the PR-shaped fields — and defines no finding, severity, or +decision rule of its own. + +## What it is not + +- **Not a GitHub status or check.** A pass/fail commit status for the + reviewed SHA is a different mechanism, owned by + [`review-status-enforcement.md`](review-status-enforcement.md). Emitting + this document never publishes, alters, or substitutes for that status. +- **Not a review body or inline comment.** It is returned to the caller + only and is never an input to the publication payload built by + [`review-output.md`](review-output.md), "Batched review construction and + submission" — the same structural boundary as the Reviewer Brief. +- **Not a new analysis phase.** Composing it changes no finding, severity, + coverage signal, or verdict, and it runs in every publication mode + (PASSIVE, SEMI, ACTIVE), whether or not anything is published. + +## Activation + +Off by default. Emitted only when the caller explicitly asks for a +machine-readable, structured, or JSON review result (for example "also +return the review as JSON"). It is additive: the human-readable report, +the Reviewer Brief, and any publication are unchanged. + +## Field population + +Shared fields (`schema_version`, `findings`, `counts`, `coverage`, +`summary`, and each finding's fields) are populated exactly as +`local-code-review` populates them, from the finalized findings and the +shared [`finding.md`](../../../shared/templates/finding.md) contract. The +PR-specific values are: + +| Field | Value for a PR review | +| --- | --- | +| `skill` | `github-pr-review` | +| `reviewed_state.repository` | the PR's base repository as `owner/name` | +| `reviewed_state.base_branch` | the PR's base branch name (the effective review base for a stacked PR) | +| `reviewed_state.base_sha` | the base SHA at review time | +| `reviewed_state.merge_base_sha` | the merge base of base and head at review time | +| `reviewed_state.reviewed_head_sha` | the exact PR head SHA the review analyzed and revalidated ([`review-output.md`](review-output.md), "HEAD revalidation") — never the current tip if it moved afterward | +| `reviewed_state.reviewer_identity` | the authenticated review identity, or `null` when it could not be established | +| `reviewed_state.completeness` | `full`, or `delta-re-review` when [`reviewer-delta-review.md`](reviewer-delta-review.md) applied | +| `reviewed_state.prior_reviewed_sha` | the same-reviewer prior reviewed SHA a delta review superseded; `null` at a chain root | +| `decision.derived` | mechanically derived from the finding set: `blocking` when any unresolved P0/P1 remains, otherwise `clean` | +| `decision.outcome` | `derived`, except `incomplete` when coverage is incomplete | + +A PR always has a committed head, so `reviewed_head_sha` is never `null`; +if the head cannot be established the review is incomplete and no +document is emitted as if it were graded. + +### Decision consistency + +`decision.derived` and `decision.outcome` are computed from the same final +verdict the human output states, never separately: + +| Machine value | Human rendering | +| --- | --- | +| `clean` | `Approve` (permitted or not — see below) | +| `blocking` | `Request Changes` | +| `incomplete` (outcome only) | `REVIEW INCOMPLETE` | + +The document records the derived decision, not the GitHub event that was +submitted. A self-review, a PASSIVE run, or an unauthorized `APPROVE` +still yields `clean` when the findings are clean; the event actually +posted is reported by the human output, not encoded here. `clean` never +means "no findings" — P2 findings still appear in `findings`. + +## Delivery + +One JSON document in a fenced `json` block, in a distinct part of the +returned result appended after the caller-facing report, never merged into +GitHub-shaped content. The final human-facing summary remains the last +review-owned publication of an active run +([`review-output.md`](review-output.md), "Submission ordering"); this +document is returned to the caller, not published, so it does not change +that ordering. diff --git a/tests/policy/review/findings/test_structured_output_docs.py b/tests/policy/review/findings/test_structured_output_docs.py new file mode 100644 index 00000000..577febca --- /dev/null +++ b/tests/policy/review/findings/test_structured_output_docs.py @@ -0,0 +1,49 @@ +#!/usr/bin/env python3 +"""Documentation-contract checks for github-pr-review structured output (Issue #70). + +Pins that the packaged policy populates every PR-shaped schema field, uses +the schema's own enum values, and stays off the publication path. + +Run with: + python3 -m unittest tests.policy.review.findings.test_structured_output_docs +""" + +from __future__ import annotations + +import json +import unittest + +from tests.support.paths import REPO_ROOT + +POLICY = REPO_ROOT / "skills" / "github-pr-review" / "policies" / "structured-output.md" +OUTPUT_POLICY = REPO_ROOT / "skills" / "github-pr-review" / "policies" / "review-output.md" +SCHEMA = REPO_ROOT / "docs" / "review-result" / "review-result.schema.json" + + +class StructuredOutputPolicyTests(unittest.TestCase): + def setUp(self) -> None: + self.text = POLICY.read_text(encoding="utf-8") + self.schema = json.loads(SCHEMA.read_text(encoding="utf-8")) + + def test_every_reviewed_state_field_is_populated(self) -> None: + for key in self.schema["properties"]["reviewed_state"]["required"]: + self.assertIn(f"`reviewed_state.{key}`", self.text) + + def test_decision_fields_and_skill_value_are_populated(self) -> None: + for token in ("`decision.derived`", "`decision.outcome`", "`skill`"): + self.assertIn(token, self.text) + self.assertIn("github-pr-review", self.schema["properties"]["skill"]["enum"]) + + def test_decision_values_match_schema_enums(self) -> None: + decision = self.schema["properties"]["decision"]["properties"] + for value in decision["outcome"]["enum"]: + self.assertIn(f"`{value}`", self.text) + + def test_not_a_commit_status_and_not_published(self) -> None: + self.assertIn("review-status-enforcement.md", self.text) + self.assertIn("never an input to the publication payload", self.text) + self.assertNotIn("structured-output", OUTPUT_POLICY.read_text(encoding="utf-8")) + + +if __name__ == "__main__": + unittest.main() From 0c14e6e6e1c8074ba1df49d366054a54d9012fdb Mon Sep 17 00:00:00 2001 From: amirbena Date: Thu, 24 Sep 2026 16:11:31 +0300 Subject: [PATCH 2/2] Address review: inline result shape, runbook steps, incomplete-head rule, drift tests (#70) Co-Authored-By: Claude Sonnet 5 --- docs/review-result/README.md | 4 +- docs/review-result/review-result-model.md | 7 ++- .../policies/structured-output.md | 55 ++++++++++++++++--- .../runbooks/active-pr-review.md | 5 ++ .../runbooks/passive-pr-review.md | 4 ++ .../findings/test_structured_output_docs.py | 16 ++++++ 6 files changed, 77 insertions(+), 14 deletions(-) diff --git a/docs/review-result/README.md b/docs/review-result/README.md index 6d69d32a..39317967 100644 --- a/docs/review-result/README.md +++ b/docs/review-result/README.md @@ -7,8 +7,8 @@ Schema and example for the machine-readable form of one review's output. Like [`../review-telemetry/README.md`](../review-telemetry/README.md) and [`../finding-confidence/README.md`](../finding-confidence/README.md), these are repository-development docs: **not** packaged into either Skill -archive, and no packaged Skill resource depends on them. `github-pr-review` can emit a -review result on explicit request ([#70](https://github.com/amirbena/code-review-skill/issues/70), +archive, and no packaged Skill resource depends on them. +`github-pr-review` can emit a review result on explicit request ([#70](https://github.com/amirbena/code-review-skill/issues/70), [`structured-output.md`](../../skills/github-pr-review/policies/structured-output.md)); `local-code-review` emission is [#69](https://github.com/amirbena/code-review-skill/issues/69). diff --git a/docs/review-result/review-result-model.md b/docs/review-result/review-result-model.md index 868547b4..e71a3a94 100644 --- a/docs/review-result/review-result-model.md +++ b/docs/review-result/review-result-model.md @@ -14,8 +14,8 @@ this record is the bug. Not packaged: no packaged Skill resource depends on this record, the schema, or the example (see [`../../AGENTS.md`](../../AGENTS.md), "Packaged -Skills are independent of repository-level instructions"). `github-pr-review` emits -one on explicit request; local emission is pending — see section 7. +Skills are independent of repository-level instructions"). +`github-pr-review` emits one on explicit request; local emission is pending — see section 7. ## 1. Files @@ -128,6 +128,7 @@ caller only, with PR-specific field population owned by its [`structured-output.md`](../../skills/github-pr-review/policies/structured-output.md) ([#70](https://github.com/amirbena/code-review-skill/issues/70)). Until [#69](https://github.com/amirbena/code-review-skill/issues/69) lands, -`local-code-review` still produces only its existing Markdown output, and [`finding.md`](../../shared/templates/finding.md)'s note that a +`local-code-review` still produces only its existing Markdown output, and +[`finding.md`](../../shared/templates/finding.md)'s note that a machine-readable renderer would be "another projection of the same fields" is what this schema is the first instance of. diff --git a/skills/github-pr-review/policies/structured-output.md b/skills/github-pr-review/policies/structured-output.md index 75dee690..05b9399e 100644 --- a/skills/github-pr-review/policies/structured-output.md +++ b/skills/github-pr-review/policies/structured-output.md @@ -7,12 +7,13 @@ Canonical semantics for the optional machine-readable review result the same finalized analysis result and is composed only once findings, severity, coverage, and the verdict are final. -The document is the same review-result schema `local-code-review` emits -(this repository's `docs/review-result/`, a repository-development record -that is not packaged): identical field names, enums, and meaning. This -policy states only what is specific to a pull request — which values -populate the PR-shaped fields — and defines no finding, severity, or -decision rule of its own. +The document is the same review-result document `local-code-review` emits: +identical field names, enums, and meaning. The shape is stated in +"Document shape" below so it is usable from the packaged Skill alone (this +repository's `docs/review-result/` holds the JSON Schema it mirrors; it is +not packaged). Beyond that shape, this policy states only what is specific +to a pull request — which values populate the PR-shaped fields — and +defines no finding, severity, or decision rule of its own. ## What it is not @@ -35,6 +36,42 @@ machine-readable, structured, or JSON review result (for example "also return the review as JSON"). It is additive: the human-readable report, the Reviewer Brief, and any publication are unchanged. +## Document shape + +One JSON object. Keys are `snake_case`; required keys are always present; +an optional finding field with no value is omitted, never `null`. + +```json +{ + "schema_version": "1.0.0", + "skill": "github-pr-review", + "reviewed_state": { + "repository": "owner/name", "base_branch": "main", + "base_sha": "", "merge_base_sha": "", + "reviewed_head_sha": "", "reviewer_identity": "", + "completeness": "full | delta-re-review", "prior_reviewed_sha": "" + }, + "coverage": "complete | incomplete", + "decision": { "derived": "clean | blocking", + "outcome": "clean | blocking | incomplete" }, + "counts": { "p0": 0, "p1": 0, "p2": 0 }, + "summary": "", + "findings": [ { "id": "F1", "severity": "P0 | P1 | P2", "title": "", + "location": "", "fix_location_resolved": true, "evidence": "", + "impact": "", "fix": "", + "runtime_validation": "reasoned | runtime-confirmed | attempted-inconclusive", + "confidence": "confirmed | credible | runtime-validation-unavailable | external-contract-unvalidated | insufficient-context", + "identity": { "stable_id": "fid_v1_", "matching_eligible": true } } ] +} +``` + +Optional finding fields, each from [`finding.md`](../../../shared/templates/finding.md) +and omitted when empty: `evidence_location`, `affected_locations`, +`follow_up`, `details`, `contextual_evidence`, `capability`, `defect_kind`. +`fix_location_resolved` is `false` exactly when the finding carries the +"evidence location; fix/action location unresolved" annotation. `counts` +equals the tally of `findings` by severity; no other key is permitted. + ## Field population Shared fields (`schema_version`, `findings`, `counts`, `coverage`, @@ -57,9 +94,9 @@ PR-specific values are: | `decision.derived` | mechanically derived from the finding set: `blocking` when any unresolved P0/P1 remains, otherwise `clean` | | `decision.outcome` | `derived`, except `incomplete` when coverage is incomplete | -A PR always has a committed head, so `reviewed_head_sha` is never `null`; -if the head cannot be established the review is incomplete and no -document is emitted as if it were graded. +If the head cannot be established the review is incomplete: emit +`coverage: incomplete`, `decision.outcome: incomplete`, and +`reviewed_head_sha: null` — never a graded outcome. ### Decision consistency diff --git a/skills/github-pr-review/runbooks/active-pr-review.md b/skills/github-pr-review/runbooks/active-pr-review.md index 0ce10a63..11d6b9ef 100644 --- a/skills/github-pr-review/runbooks/active-pr-review.md +++ b/skills/github-pr-review/runbooks/active-pr-review.md @@ -640,6 +640,11 @@ stop body, per [`../policies/reviewer-brief.md`](../policies/reviewer-brief.md), "Composition with invocation modes" — never its fields or boundaries. + **When the caller explicitly requested a machine-readable result**, + compose it here too, from the same finalized result, per + [`../policies/structured-output.md`](../policies/structured-output.md); + like the brief it joins the returned result only and is never passed + into steps 15-16. 14. **Apply the review-action authorization gate** per [`../policies/review-action-authorization.md`](../policies/review-action-authorization.md) and [`../policies/review-output.md`](../policies/review-output.md), diff --git a/skills/github-pr-review/runbooks/passive-pr-review.md b/skills/github-pr-review/runbooks/passive-pr-review.md index 9e14d2c9..1225c3da 100644 --- a/skills/github-pr-review/runbooks/passive-pr-review.md +++ b/skills/github-pr-review/runbooks/passive-pr-review.md @@ -433,6 +433,10 @@ finally: remove the temporary checkout (success, any failure, interruption) field, synthesis, and mode-composition rule in that policy (clean review, delta re-review, stacked PR, partitioned large PR alike) so passive and active results carry identical brief semantics. + **When the caller explicitly requested a machine-readable result**, + also compose it here, from the same finalized result, per + [`../policies/structured-output.md`](../policies/structured-output.md); + it is returned to the caller only. 8e. **Check verdict consistency** per [`../../../shared/policies/verdict-consistency.md`](../../../shared/policies/verdict-consistency.md) before returning the report composed above: confirm the decision diff --git a/tests/policy/review/findings/test_structured_output_docs.py b/tests/policy/review/findings/test_structured_output_docs.py index 577febca..0545a927 100644 --- a/tests/policy/review/findings/test_structured_output_docs.py +++ b/tests/policy/review/findings/test_structured_output_docs.py @@ -18,6 +18,7 @@ POLICY = REPO_ROOT / "skills" / "github-pr-review" / "policies" / "structured-output.md" OUTPUT_POLICY = REPO_ROOT / "skills" / "github-pr-review" / "policies" / "review-output.md" SCHEMA = REPO_ROOT / "docs" / "review-result" / "review-result.schema.json" +EXAMPLE = REPO_ROOT / "docs" / "review-result" / "examples" / "review-result.example.json" class StructuredOutputPolicyTests(unittest.TestCase): @@ -44,6 +45,21 @@ def test_not_a_commit_status_and_not_published(self) -> None: self.assertIn("never an input to the publication payload", self.text) self.assertNotIn("structured-output", OUTPUT_POLICY.read_text(encoding="utf-8")) + def test_inline_shape_matches_schema_and_example(self) -> None: + finding = self.schema["definitions"]["finding"] + for key in finding["required"] + list(self.schema["required"]): + self.assertIn(f'"{key}"', self.text) + for key in finding["properties"]: + self.assertIn(f"`{key}`" if key not in finding["required"] else f'"{key}"', self.text) + example = json.loads(EXAMPLE.read_text(encoding="utf-8")) + self.assertEqual(set(example["reviewed_state"]), set(self.schema["properties"]["reviewed_state"]["required"])) + self.assertEqual(example["skill"], "github-pr-review") + + def test_runbooks_compose_structured_output(self) -> None: + for name in ("active-pr-review.md", "passive-pr-review.md"): + runbook = (POLICY.parents[1] / "runbooks" / name).read_text(encoding="utf-8") + self.assertIn("policies/structured-output.md", runbook) + if __name__ == "__main__": unittest.main()