feat(review): which lenses gate a pull request is a decision of its own

.lockstep/lockstep.py

@@ -687,4 +687,11 @@
 judge_workflows.register()
 # `review/all-lenses`: the required check, as one fan-out over every lens the bound adapter
 # declares (GATE-REVIEW-5, GATE-COST-6). `lockstep.yml` invokes it and nothing else.
+#
+# No `gating=`, and that is a decision rather than an omission. Since #449 a repository can
+# name which of its bound lenses gate a pull request, leaving the rest bound and reachable
+# from `/review <lens>` on the thread but off the required check. THIS repository may not:
+# O10 exists so the four shipped lenses are not things we ask adopters to trust on our word,
+# and `intent`, `performance` and `tests` already spent a release shipped-but-never-run once
+# (#241). `GATE-REVIEW-9` fails the build if an argument appears here.
 review_workflows.register()

design/objectives.md

@@ -94,8 +94,8 @@ to move without somebody standing in front of this table.
 | `O5` | The record is what teaches it | held | `GATE-IMPROVE-1`, `GATE-IMPROVE-2`, `GATE-IMPROVE-3`, `GATE-IMPROVE-4`, `GATE-IMPROVE-5`, `GATE-IMPROVE-6`, `GATE-IMPROVE-7`, `GATE-IMPROVE-8`, `GATE-EVAL-2`, `GATE-EVAL-4`, `GATE-EVIDENCE-1`, `GATE-OUT-2`, `GATE-LEDGER-12`, `GATE-JUDGE-1`, `GATE-JUDGE-3`, `GATE-LEDGER-2`, `GATE-LEDGER-4`, `GATE-EVAL-1`, `GATE-EVAL-3`, `GATE-REVIEW-7` | — | The reading half, the writing half and, since PR-18, the half that reads back: `improve` reads the ledger for a qualifying trend, drafts a change to the one declared body that trend is attributed to, measures the draft against the promoted corpus on both arms -- the bound judge settling the rubrics, a verdict kept beside its case -- opens the change with its scorecard, and `report --around` compares the runs after the merge with the runs before it on that body's subject (`GATE-LEDGER-2`). What this repository's own ledger can show of that last step was thin when the row moved -- the Phase 4 merge had two subject-carrying runs after it, because the required check's fan-out records carried its lenses as steps without a subject -- and the fix that followed stamps each lens step with its subject and its bill, so every check since is four comparable runs; the records before it stay thin and the window says so. On this repository the corpus is one promoted case with one kept verdict, so the loop refuses before spending until somebody tightens a case to what a correct answer would have said -- the labelling act it learns from. The reason the first dispatched `improve` run actually gives belongs in this row once it has run. |
 | `O6` | The model never holds a secret | held | `GATE-AUTH-1`, `GATE-AUTH-2`, `GATE-SANDBOX-1`, `GATE-SANDBOX-2`, `GATE-EGRESS-1`, `GATE-EGRESS-2`, `GATE-EGRESS-3`, `GATE-REDACT-1`, `GATE-REDACT-2`, `GATE-REDACT-3`, `GATE-GUARD-4`, `GATE-CFG-1`, `GATE-POLICY-2`, `GATE-CFG-2`, `GATE-RETRY-6`, `GATE-APPROVAL-1`, `GATE-GUARD-1`, `GATE-GUARD-2`, `GATE-DELEGATE-1`, `GATE-SEARCH-1`, `GATE-CFG-5` | — | The container a model-staged test needs is one whose image carries the suite's dependencies, and `init` names that image rather than deriving it, so an adopter's first `implement` or `fix` refuses `sandbox.host_fallback` until a person writes the line (`GATE-SANDBOX-2` says why a stack image will not do). This repository's own container mounts the `.venv` its runner built, which runs the suite wherever its packages import on linux -- everywhere, while they stay pure Python -- and fails at import, naming the module, the day one does not. Egress for the run as a whole stays opted out here and in the scaffold (`GATE-EGRESS-2`, #315). |
 | `O7` | Determinism first | held | `GATE-REVIEW-2`, `GATE-REVIEW-3`, `GATE-REVIEW-4`, `GATE-EVAL-4`, `GATE-COST-3`, `GATE-SHAPE-1`, `GATE-VERDICT-1`, `GATE-PROGRESS-1`, `GATE-OUT-3`, `GATE-JUDGE-2`, `GATE-WORKSPACE-1`, `GATE-VALIDATE-2`, `GATE-ASSESS-1` | — | — |
-| `O8` | Extended without forking | held | `GATE-PACK-1`, `GATE-PACK-2`, `GATE-PACK-3`, `GATE-PACK-4`, `GATE-PACK-5`, `GATE-PLUGIN-1`, `GATE-PLUGIN-2`, `GATE-PLUGIN-3`, `GATE-DOCS-1`, `GATE-BODY-1`, `GATE-POLICY-1` | — | — |
-| `O9` | New aspects on a verb that already exists | held | `GATE-REVIEW-3`, `GATE-PACK-5`, `GATE-REVIEW-5`, `GATE-LENS-1`, `GATE-BLAST-1` | — | — |
+| `O8` | Extended without forking | held | `GATE-PACK-1`, `GATE-PACK-2`, `GATE-PACK-3`, `GATE-PACK-4`, `GATE-PACK-5`, `GATE-PLUGIN-1`, `GATE-PLUGIN-2`, `GATE-PLUGIN-3`, `GATE-DOCS-1`, `GATE-BODY-1`, `GATE-POLICY-1`, `GATE-REVIEW-9` | — | — |
+| `O9` | New aspects on a verb that already exists | held | `GATE-REVIEW-3`, `GATE-PACK-5`, `GATE-REVIEW-5`, `GATE-REVIEW-9`, `GATE-LENS-1`, `GATE-BLAST-1` | — | — |
 | `O10` | It runs on itself | held | `GATE-CI-1`, `GATE-RECORD-1`, `GATE-TEST-3`, `GATE-REVIEW-5`, `GATE-CFG-3`, `GATE-CI-3`, `GATE-DOGFOOD-1`, `GATE-LEDGER-10`, `GATE-CI-5`, `GATE-VALIDATE-1` | — | Reviews on every pull request, a review asked for on a thread, a fix and measurement have closed here (`GATE-DOGFOOD-1` names the runs); an implementation has not, and neither has delegation: `.lockstep/lockstep.py` binds `TDD(delegation=True)`, so the next `/implement` here holds `delegate` (`GATE-DELEGATE-1`), and no run has used it yet. Deliberately not dogfooded on this repository's own runs, and each a separate decision: egress enforcement and the daily ceiling (off, documented), GitLab (no instance; O3 says so), and any provider but Anthropic (`GATE-COST-3` prices it; nothing here has routed to it). The required `review` check is enforced by choice -- `doctor` now says so (`DOC127`, `DOC128`) -- and stays that way while one engineer is the administrator. |
 | `O11` | The provider and the model are the adopter's, not ours | held | `GATE-AUTH-2`, `GATE-RESIDENCY-1`, `GATE-COST-4`, `GATE-LENS-1`, `GATE-MODEL-1`, `GATE-JUDGE-2`, `GATE-COST-3`, `GATE-ASSESS-2` | — | — |
 | `O12` | A second engineer is served, not obstructed | held | `GATE-TEAM-1`, `GATE-TEAM-2`, `GATE-LEDGER-10`, `GATE-LEDGER-11`, `GATE-LEDGER-12`, `GATE-REVIEW-6`, `GATE-OUT-6`, `GATE-LEDGER-1`, `GATE-LEDGER-3`, `GATE-LEDGER-7`, `GATE-LEDGER-8`, `GATE-LEDGER-9`, `GATE-REVIEW-1`, `GATE-VERDICT-2`, `GATE-RECORD-6`, `GATE-FORK-1`, `GATE-REPORT-1`, `GATE-CONTEXT-1`, `GATE-CFG-4`, `GATE-REVIEW-8` | — | — |

docs/extending.md

@@ -159,6 +159,34 @@ aspect then reports the lenses *this adapter* has, not the ones that happen to s
 The map is copied at construction, in both directions: a later mutation of `LENSES` cannot reach
 an adapter you already bound, and an adapter cannot leak a lens back into the shipped map.
 
+### Which of them gate a pull request
+
+That map says which lenses this repository **has**, and `/review <lens>` on a thread resolves
+against exactly it — so narrowing it to keep a lens off the required check would also put that lens
+out of reach of the comment asking for it. Which lenses **gate** is a different decision, and it
+sits on the registration of the check rather than on the adapter, because that is what it is a
+property of:
+
+```python
+from in_lockstep.workflows import review as review_workflows
+
+review_workflows.register(gating=("security", "tests"))
+```
+
+Five lenses bound, two named: the required check runs those two and pays for those two, and the
+other three are still one `/review performance` away on any thread. Omit the argument — as this
+repository does, deliberately — and every bound lens gates.
+
+A name the bound adapter does not declare refuses the run, listing it beside the set that exists,
+before the fan-out and therefore before anything spends. `gating=()` is refused separately, because
+"nothing gates" and "everything gates" are opposite intentions and the second is already what an
+omitted argument means. Both refusals exit 3, so a selection with a typo in it is a red check
+rather than a green one over nothing.
+
+There is deliberately no flag for this. The selection is executable configuration in the one file
+you are meant to edit, never an argument in a workflow file: a lens list in YAML is precisely the
+thing that goes stale, which is why the check stopped taking one.
+
 ### Enhancing a shipped lens, without a subclass
 
 An entry in that map is either a prompt class or a `Lens`: the declared form, carrying the prompt

src/in_lockstep/workflows/review.py

@@ -1,4 +1,4 @@
-"""The required review check as one run: every lens the bound adapter declares, fanned out.
+"""The required review check as one run: the lenses that gate a pull request, fanned out.
 
 `review/all-lenses` is what `lockstep.yml` invokes on every pull request here. Until Phase 5 the
 check was one `review` invocation looping four `--aspect` flags in sequence, and the file named
@@ -9,12 +9,25 @@
 branch's findings are the run's findings, and one record with four steps is written where four
 records were.
 
+Since #449 those branches are the lenses that GATE, which is not the same set as the lenses that
+EXIST. `AiReview(lenses=...)` answers the second and is what `/review <lens>` on a thread resolves
+against; `register(gating=...)` answers the first. One map used to answer both, so the only lever
+for keeping a lens off the required check was dropping it -- which also put it out of reach of the
+comment asking for it. The two are orthogonal now: a lens bound but not gating is available on the
+thread and not run by the check.
+
+Here all four gate, by decision and not by default. O10 exists so the shipped lenses are not
+things we ask adopters to trust on our word, and a subset in THIS repository would be that defect
+returning (`GATE-REVIEW-9`).
+
 A lens's comment body is still one file per lens under the directory the trampoline names, so
-the `publish` job posts four sticky comments a reader can tell apart (`GATE-REVIEW-6`).
+the `publish` job posts one sticky comment per gating lens a reader can tell apart
+(`GATE-REVIEW-6`).
 """
 
 from __future__ import annotations
 
+from collections.abc import Collection
 from typing import Any
 
 from ..adapters.ai.review import Review
@@ -39,32 +52,81 @@ def bound_lenses(ctx: RunContext) -> tuple[str, ...]:
     return tuple(sorted(str(label).rsplit("/", 1)[-1] for label in compositions()))
 
 
+def _named(names: Collection[str]) -> str:
+    return ", ".join(sorted(names))
+
+
+def _refused(reason: str, message: str) -> Outcome[Any]:
+    """A configuration this check will not guess at, as the run's own outcome.
+
+    `blocked` rather than `failed`, because nothing ran and nothing spent and that is §4.3's own
+    category. It is still red where it matters: `EXIT_BLOCKED` is 3, so a required check whose
+    lens selection is wrong fails rather than reading as a green check that reviewed nothing.
+    """
+    return Outcome.blocked_by(
+        reason,
+        findings=(Finding(id=reason, message=message, severity=Severity.ERROR, blocking=True),),
+    )
+
+
 async def review_all_lenses(
-    ctx: RunContext, base: str, head: str, comments: str = "", diff: str = ""
+    ctx: RunContext,
+    base: str,
+    head: str,
+    comments: str = "",
+    diff: str = "",
+    *,
+    gating: Collection[str] | None = None,
 ) -> Outcome[Any]:
-    """Every lens over one change, at once, as one run.
+    """Every lens that gates this change, at once, as one run.
 
     `comments` is a directory: one body per lens lands in it for the job that holds the write
     token to post. `diff` is for a replay whose tape was recorded against a diff git no longer
     has (the shipped fixture), and is otherwise empty.
+
+    `gating` comes from `register(gating=...)` and from nowhere else. Keyword-only, and absent
+    from the callable that `register` actually registers, because `in-lockstep run` builds its
+    `--arg` surface by introspecting that callable's signature -- and a lens list reachable from
+    the trampoline is the question `GATE-REVIEW-5` closed.
     """
-    lenses = bound_lenses(ctx)
-    if not lenses:
-        return Outcome.blocked_by(
+    declared = bound_lenses(ctx)
+    if not declared:
+        return _refused(
             "review.no_lenses",
-            findings=(
-                Finding(
-                    id="review.no_lenses",
-                    message=(
-                        "no Review adapter declaring its lenses is bound; nothing was reviewed. "
-                        "`in-lockstep run` binds the shipped AiReview when a module binds none, so "
-                        "this means a module bound an adapter with no `compositions()`."
-                    ),
-                    severity=Severity.ERROR,
-                    blocking=True,
-                ),
-            ),
+            "no Review adapter declaring its lenses is bound; nothing was reviewed. "
+            "`in-lockstep run` binds the shipped AiReview when a module binds none, so this "
+            "means a module bound an adapter with no `compositions()`.",
         )
+
+    lenses = declared
+    if gating is not None:
+        # Resolved against what the adapter declares rather than trusted, and resolved HERE --
+        # after the binding exists, before the fan-out. `register` cannot do it: a module is free
+        # to call it before it binds `Review`, and there is no container at registration time.
+        asked = tuple(dict.fromkeys(gating))
+        if not asked:
+            return _refused(
+                "review.gating_empty",
+                "`register(gating=...)` names no lens at all, so nothing would gate and this "
+                "check could not fail for any reason. Omit the argument to gate on every bound "
+                f"lens ({_named(declared)}), or name the ones that should.",
+            )
+        unknown = tuple(name for name in asked if name not in declared)
+        if unknown:
+            return _refused(
+                "review.gating_unknown",
+                f"`register(gating=...)` names {_named(unknown)}, which the bound Review adapter "
+                f"does not declare. It declares {_named(declared)}. Refused rather than dropped: "
+                "a typo that quietly gates on fewer lenses than somebody meant is a check that "
+                "goes green for the wrong reason, which is the one failure a required check must "
+                "not have.",
+            )
+        lenses = tuple(name for name in declared if name in set(asked))
+        # Said out loud on every run, because the difference between the bound set and the gating
+        # set is invisible in the check's output otherwise -- and a reader looking at four sticky
+        # comments where five lenses are bound should not have to open `lockstep.py` to know why.
+        print(f"gating    {len(lenses)} of {len(declared)} bound lenses: {_named(lenses)}")
+
     join = await ctx.fan_out(
         branches={
             lens: ctx.call(Review(base=base, head=head, aspect=lens, diff=diff), step=lens) for lens in lenses
@@ -81,9 +143,38 @@ async def review_all_lenses(
     return join.as_outcome()
 
 
-def register() -> None:
-    """Claim the `review/*` ids. Called by an adopter's `lockstep.py`, never on import."""
-    workflow(id=ALL_LENSES)(review_all_lenses)
+def register(*, gating: Collection[str] | None = None) -> None:
+    """Claim the `review/*` ids, and say which of the bound lenses gate a pull request.
+
+    Called by an adopter's `lockstep.py`, never on import.
+
+    `gating` lives here and not beside `AiReview(lenses=...)` because it is a property of the
+    CHECK rather than of the adapter. The adapter's map says which lenses this repository HAS, and
+    `chatops.aspect_from` resolves `/review <lens>` against exactly that map -- so narrowing it to
+    keep a lens off the required check would also put that lens out of reach of the comment asking
+    for it. That is the lever #449 was filed about being the wrong one, and this is the right one.
+
+    `None`, the default, gates on every bound lens: a repository that says nothing is unaffected.
+    A named lens the bound adapter does not declare refuses the run by name, at run time and
+    before the fan-out, so nothing has spent.
+    """
+    selection = None if gating is None else tuple(gating)
+
+    async def all_lenses(
+        ctx: RunContext, base: str, head: str, comments: str = "", diff: str = ""
+    ) -> Outcome[Any]:
+        return await review_all_lenses(ctx, base, head, comments, diff, gating=selection)
+
+    # Re-declared rather than wrapped with `functools.wraps`. `wraps` sets `__wrapped__` and
+    # `inspect.signature` follows it, which would put `gating` among the arguments `in-lockstep
+    # run` accepts as `--arg` -- a lens list reachable from the trampoline YAML, i.e. exactly what
+    # `GATE-REVIEW-5` closed. The four parameters above are the whole callable surface.
+    #
+    # `__qualname__` is left as the closure's own: two calls read as ONE declaration to
+    # `_same_declaration` (same module, same qualname), so re-executing a `lockstep.py` reinstalls
+    # its selection rather than raising `DuplicateWorkflow` against itself.
+    all_lenses.__doc__ = review_all_lenses.__doc__
+    workflow(id=ALL_LENSES)(all_lenses)
 
 
 __all__ = ["ALL_LENSES", "bound_lenses", "register", "review_all_lenses"]