Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions .github/PULL_REQUEST_TEMPLATE.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,42 @@ concrete gap and consumer. These author facts are independently reviewed.
- Observable before → after, with the validation row that proves it:
- Issue/task and intended base: <!-- Use Closes only for the issue actually completed; otherwise Related to. -->

## Author Declaration

<!--
Who wrote this change, and what it was implemented against. A reviewer may be a
different operator's agent, working from a different host and context: this
section is the only place it can learn both without guessing, and unlike the
rest of the body it is published attribution rather than a reviewed claim. Keep
it to one short line plus the spec rows; do not paste runtime identifiers,
endpoints, credentials or internal routing detail, and never claim a
verification you did not run.
-->

- Written by: <!-- model_agent or human_operator; for an agent, the model and provider in ordinary product-family wording; keep it to one line -->

### Implemented against

<!-- The accepted specification this change was built for: an accepted RFC,
accepted contract/protocol document, the linked issue or task, or a
maintainer-agreed review frame. Give the exact file path or public link and the
revision, so a reviewer can open the same text. One row per criterion the
specification states, using its own section or identifier when it has one.
Disposition: implemented | deferred | out_of_scope | not_met — a not_met row
blocks approval and needs its gap and repair. Write "no written specification;
the request in this PR is the basis" when none exists. Ignore aspirational or
future properties; they are not obligations.
-->

- Specification and revision:
- Criteria:

| Criterion (spec clause) | Disposition | Symbol / path | Test or command |
| --- | --- | --- | --- |
| | | | |

- Self-check before submission: <!-- What you ran, what you read, and what you deliberately left out. Separate what you verified from what you assumed; "none" with a reason is valid. -->

## Scope And Continuation

<!-- A scoped fix may be complete while the parent program remains open.
Expand Down
24 changes: 24 additions & 0 deletions loopx/capabilities/pr_review_queue/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -442,6 +442,30 @@ them. Queue selection, scheduler and runtime permissions are unchanged. The
five public sections retain the bounded prose floors documented above; reuse
evidence and do not pad. Invoking review uses the installed capability policy.

### Reviewer provenance and specification basis

Policy revision 14 makes two publication requirements mandatory for every new
result; neither changes queue selection, merge authority or how already
published GitHub reviews are recognized (`check_review_body` is unchanged).

- `result.reviewer` (`reviewer_declaration`) names `actor_kind`
(`model_agent` or `human_operator`) and `declaration_source`
(`runtime_reported` or `self_reported`); an agent adds `declared_model` and
`declared_provider` in product-family wording. The body repeats them on
exactly one visible `Reviewer:` line, each value as a whole token. This is
provenance, not a credential: it authenticates nothing and adds no weight.
- `problem_context.spec_basis` (`spec_basis_assessment`) is `mapped`,
`no_spec` or `not_yet_proven`. `mapped` gives text `spec_ref`,
`spec_revision` (a full commit id for `accepted_rfc` and
`accepted_contract_doc`) and one criterion row per material acceptance
criterion; `not_met` and `not_yet_proven` block approval. The reference,
revision and every `criterion_id` must appear as whole tokens in the
visible body, because another operator reads only the published review.

`--check-result` reports a missing or inconsistent declaration or unpublished
reference as an error, and a spec-basis gap as an approval blocker. Regenerate
results made under revision 13 rather than adding these fields to them.

### Semantic alignment and CI constraint recovery

The semantic triage introduced in policy revision 5 replaces universal detailed
Expand Down
152 changes: 151 additions & 1 deletion loopx/capabilities/pr_review_queue/result_check.py
Original file line number Diff line number Diff line change
@@ -1,18 +1,25 @@
"""Check a review's declared evidence for consistency, never its truth."""

import re
from collections.abc import Mapping
from typing import Any

from .review_contract import (
COMPATIBILITY_ASSESSMENT,
OUTCOME_IMPACT_ASSESSMENT,
REVIEWER_DECLARATION,
SCOPE_COVERAGE_ASSESSMENT,
SEMANTIC_CANDIDATE_DECISIONS,
SPEC_BASIS_ASSESSMENT,
VALIDATION_FAILURE_ATTRIBUTION,
build_review_execution_contract,
build_review_plan,
)
from .review_body import check_review_body
from .review_body import (
check_review_body,
reviewer_declaration_lines,
visible_review_text,
)


def _missing(value: object) -> bool:
Expand Down Expand Up @@ -206,6 +213,141 @@ def _check_outcome_impact(blockers: list[str], value: object) -> None:
fields=["acceptance_basis", "bounded_cost_and_recovery"])


# The declaration names a product family, so the characters that mark a URL,
# path, host:port, account or ARN have no place in it. This is a shape rule,
# not a product-name denylist. Known false positive: router- or
# version-qualified ids such as `vendor/model` or `model@version` are rejected;
# declare the product-family name instead. A typed runtime-reported identity
# would replace this text check once a host supplies one.
_DECLARED_NAME_FORBIDDEN = re.compile(r"[/:@]")
_PUBLISHED_LINE_FORBIDDEN = re.compile(r"://|@")


# Same full-length object ids the packet accepts for an exact head.
_COMMIT_ID = re.compile(r"[0-9a-f]{40}|[0-9a-f]{64}")


def _published_as_token(reference: str, text: str) -> bool:
# A whole-token match: EX-1 inside EX-10, or C1 inside a commit id, does not
# name the criterion to a reader.
pattern = rf"(?<![0-9a-z_-]){re.escape(reference)}(?![0-9a-z_-])"
return re.search(pattern, text) is not None


def _reviewer_errors(value: object, body: str) -> list[str]:
key = "reviewer"
contract = REVIEWER_DECLARATION
if not isinstance(value, Mapping):
return [f"{key}:missing_declaration"]
actor_kind = value.get("actor_kind")
if actor_kind not in contract["actor_kinds"]:
return [f"{key}:invalid_actor_kind"]
errors: list[str] = []
if value.get("declaration_source") not in contract["declaration_sources"]:
errors.append(f"{key}:invalid_declaration_source")
declared: list[str] = [str(actor_kind)]
if actor_kind == "model_agent":
for field in contract["model_agent_fields"]:
text = value.get(field)
if not isinstance(text, str) or not text.strip():
errors.append(f"{key}:missing_field:{field}")
elif _DECLARED_NAME_FORBIDDEN.search(text):
errors.append(f"{key}:not_a_product_family_name:{field}")
else:
declared.append(text.strip())
lines = reviewer_declaration_lines(body)
if len(lines) != 1:
errors.append(f"{key}:body_line_missing_or_ambiguous")
elif _PUBLISHED_LINE_FORBIDDEN.search(lines[0]):
errors.append(f"{key}:body_line_carries_runtime_detail")
elif any(not _published_as_token(token.casefold(), lines[0].casefold())
for token in declared):
errors.append(f"{key}:body_line_disagrees_with_declaration")
return errors


def _check_spec_basis(blockers: list[str], value: object) -> None:
key = "problem_context:spec_basis"
contract = SPEC_BASIS_ASSESSMENT
_require_fields(blockers, evidence_id=key, value=value, fields=contract["fields"])
if not isinstance(value, Mapping):
return
decision = value.get("decision")
if decision not in contract["decision_values"]:
blockers.append(f"{key}:invalid_decision")
return
source = value.get("spec_source")
if source not in contract["spec_source_values"]:
blockers.append(f"{key}:invalid_spec_source")
if decision in contract["blocking_decisions"]:
blockers.append(f"{key}:blocking_decision")
if decision == "no_spec" and source != "none":
blockers.append(f"{key}:no_spec_cannot_cite_a_source")
if decision != "mapped":
return
if source == "none":
blockers.append(f"{key}:mapped_without_spec_source")
_require_fields(blockers, evidence_id=key, value=value, fields=contract["mapped_fields"])
# The reference and revision are published and matched against the body, so
# a non-string value would pass the non-empty check and then never be
# compared; require text here, as for criterion_id below.
for field in contract["published_text_fields"]:
text = value.get(field)
if not _missing(text) and (not isinstance(text, str) or not text.strip()):
blockers.append(f"{key}:invalid_{field}")
revision = value.get("spec_revision")
if (source in contract["commit_pinned_spec_sources"] and isinstance(revision, str)
and revision.strip() and not _COMMIT_ID.fullmatch(revision.strip().lower())):
blockers.append(f"{key}:spec_revision_not_a_commit_id")
criteria = _require_items(
blockers, evidence_id=key, row=value,
requirement={"items_field": "criteria", "item_fields": contract["criterion_fields"],
"item_count": {"minimum": 1}},
)
seen: set[str] = set()
for criterion in criteria:
# Membership, hashing and publication all assume a public identity.
# A bool hashes and is non-empty, and a non-string identity silently
# falls out of the published-body check, so validate the type here
# rather than letting a later set or dict operation decide.
criterion_id = criterion.get("criterion_id")
if not isinstance(criterion_id, str) or not criterion_id.strip():
blockers.append(f"{key}:invalid_criterion_id")
continue
if criterion_id in seen:
blockers.append(f"{key}:duplicate_criterion:{criterion_id}")
seen.add(criterion_id)
disposition = criterion.get("disposition")
if not isinstance(disposition, str) or disposition not in contract["disposition_fields"]:
blockers.append(f"{key}:invalid_disposition:{criterion_id}")
continue
_require_fields(blockers, evidence_id=f"{key}:{criterion_id}", value=criterion,
fields=contract["disposition_fields"][disposition])
if disposition in contract["blocking_dispositions"]:
blockers.append(f"{key}:unmet_criterion:{criterion_id}")


def _unpublished_spec_references(value: object, body: str) -> list[str]:
# The structured result stays local; another operator reads only the body.
# A path alone moves with the branch, so the immutable revision the review
# judged against has to travel with it for that reader to open the same text.
if not isinstance(value, Mapping) or value.get("decision") != "mapped":
return []
text = visible_review_text(body).casefold()
references = [value.get("spec_ref"), value.get("spec_revision")]
criteria = value.get("criteria")
if isinstance(criteria, list):
references += [item.get("criterion_id") for item in criteria if isinstance(item, Mapping)]
# Non-text identities are rejected by _check_spec_basis; they are skipped
# here only because they have no text to look for in the body.
return [
f"review_body:spec_reference_not_published:{reference}"
for reference in references
if isinstance(reference, str) and reference.strip()
and not _published_as_token(reference.strip().casefold(), text)
]


def _check_scope_coverage(blockers: list[str], value: object) -> None:
key = "observable_semantics:scope_coverage"
contract = SCOPE_COVERAGE_ASSESSMENT
Expand Down Expand Up @@ -302,6 +444,7 @@ def check_review_result(
requirement = requirements[key]
if key == "problem_context":
_check_outcome_impact(blockers, row.get("outcome_impact"))
_check_spec_basis(blockers, row.get("spec_basis"))
if key == "code_volume":
_check_compatibility_assessment(blockers, row.get("compatibility_assessment"))
if key == "observable_semantics":
Expand Down Expand Up @@ -402,6 +545,13 @@ def check_review_result(
head_oid=str(matches[0].get("head_oid") or ""),
behavior_bearing=applicability.get("behavior_bearing_change") is True)
errors.extend(f"review_body:{reason}" for reason in body["invalid_reasons"])
body_text = str(result.get("review_body") or "")
errors.extend(_reviewer_errors(result.get("reviewer"), body_text))
problem_context = evidence.get("problem_context")
errors.extend(_unpublished_spec_references(
problem_context.get("spec_basis") if isinstance(problem_context, Mapping) else None,
body_text,
))
if body["verdict"] is not None and body["verdict"] != verdict:
errors.append("review_body:verdict_mismatch")
if verdict not in {"APPROVE", "REQUEST_CHANGES"}:
Expand Down
15 changes: 15 additions & 0 deletions loopx/capabilities/pr_review_queue/review_body.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,21 @@ def _visible_lines(body: str) -> Iterator[str]:
yield visible


def visible_review_text(body: str) -> str:
return "\n".join(_visible_lines(body))


def reviewer_declaration_lines(body: str) -> list[str]:
# Same visibility rule as the English verdict: a comment or fenced block
# cannot carry the declaration a reader is meant to see.
lines: list[str] = []
for line in _visible_lines(body):
match = re.match(r"(?i)^reviewer\s*:\s*(.+)$", line.strip().replace("**", ""))
if match:
lines.append(match.group(1).strip())
return lines


def _prose_size(lines: list[str]) -> int:
prose = "\n".join(dict.fromkeys(lines))
prose = re.sub(r"!?\[([^\]]*)\]\([^)]*\)", r"\1", prose)
Expand Down
Loading
Loading