review fixes round 6: require a cited finding id to clear, strip model-supplied agent at the parse boundary, pin the hard-floor set

.expo-code-review/config.jsonc

@@ -8,7 +8,7 @@
   // Add or remove files to change the roster — no list needed here.
   // shared.md (prepended to every agent + coordinator) and coordinator.md are
   // reserved filenames. Per-agent overrides go in each file's YAML frontmatter,
-  // e.g. `---\nmodel: anthropic/claude-opus-5\n---`.
+  // e.g. `---\nmodel: anthropic/claude-opus-4-1\n---`.
 
   "policy": {
     // Phase 1: keep signal high by surfacing only critical/warning.

.github/workflows/expo-code-review.yml

@@ -16,7 +16,7 @@ concurrency:
 
 jobs:
   review:
-    runs-on: expo-large
+    runs-on: ubuntu-latest
     # Trigger policy lives in .expo-code-review/config.jsonc (review.trigger); `ecr ci`
     # self-gates on it (and honors the ai-review:skip label). This coarse gate just
     # avoids spinning up a runner for a PR that explicitly opted out. Uses the array
@@ -49,29 +49,6 @@ jobs:
 
       - uses: oven-sh/setup-bun@v2
 
-      # `ecr ci` reads the whole PR through `gh`, which GitHub-hosted images ship and
-      # ours do not. Pinned by version AND sha256: this binary runs with GH_TOKEN and
-      # the model credential in its environment, so "whatever the image happens to
-      # ship" is a worse answer than one we can name. Installs into $HOME, never the
-      # workspace — resolveTrustedTool refuses a `gh` found inside the reviewed tree.
-      - name: Install gh
-        env:
-          GH_VERSION: "2.97.0"
-          GH_SHA256: "a2c9b8497e1f85b1ad0dfcb78b5a622e098801b8e461e459e88e1ee12f018112"
-        run: |
-          set -euo pipefail
-          tarball="gh_${GH_VERSION}_linux_amd64.tar.gz"
-          curl -fsSL --retry 3 -o "$RUNNER_TEMP/$tarball" \
-            "https://github.com/cli/cli/releases/download/v${GH_VERSION}/${tarball}"
-          echo "${GH_SHA256}  $RUNNER_TEMP/$tarball" | sha256sum -c -
-          mkdir -p "$HOME/.local/bin"
-          tar -xzf "$RUNNER_TEMP/$tarball" -C "$RUNNER_TEMP"
-          install -m 0755 "$RUNNER_TEMP/gh_${GH_VERSION}_linux_amd64/bin/gh" "$HOME/.local/bin/gh"
-          echo "$HOME/.local/bin" >> "$GITHUB_PATH"
-
-      - name: Check gh is available
-        run: gh --version
-
       - name: Install dependencies
         run: bun install --frozen-lockfile
 

.github/workflows/ref-check.yml

@@ -10,16 +10,10 @@ permissions:
 
 jobs:
   ref-check:
-    runs-on: expo-small
+    runs-on: ubuntu-latest
     timeout-minutes: 10
     steps:
       - uses: actions/checkout@fbc6f3992d24b796d5a048ff273f7fcc4a7b6c09 # v5
 
-      # ref-check is a python3 script. GitHub-hosted images ship python3; a
-      # self-hosted one is not guaranteed to, so ask for it explicitly.
-      - uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0
-        with:
-          python-version: "3.12"
-
       - name: Validate LLP references and metadata
         run: ./ref-check --verbose

.github/workflows/release.yml

@@ -12,7 +12,7 @@ permissions:
 
 jobs:
   publish:
-    runs-on: expo-small
+    runs-on: ubuntu-latest
     timeout-minutes: 10
     steps:
       - uses: actions/checkout@fbc6f3992d24b796d5a048ff273f7fcc4a7b6c09 # v5

.github/workflows/test.yml

@@ -10,7 +10,7 @@ permissions:
 
 jobs:
   test:
-    runs-on: expo-small
+    runs-on: ubuntu-latest
     timeout-minutes: 10
     steps:
       - uses: actions/checkout@fbc6f3992d24b796d5a048ff273f7fcc4a7b6c09 # v5

llp/0010-stack-aware-requalification.explainer.md

@@ -25,7 +25,7 @@ The IO and the logic are split on purpose. `walkUpstack` is a pure, exported fun
 
 - **Same-repo, root included** — a child counts only when `head.repo.full_name` equals `owner/repo` [observed] ([github-pr.ts](../src/sources/github-pr.ts) `fetchOpenChildren` sets `sameRepo`). The REST pulls-list endpoint has no `isCrossRepository` field, so this comparison is the fork gate; a fork PR is dropped. The ROOT PR gets the same gate before the walk even starts: a fork PR's `headRefName` names a branch of the FORK, and using it as the pulls-list `base=` filter would match a same-named base-repo branch (a fork head called `main` would pull in every open PR targeting `main`), letting unrelated PRs into the manifest — so a cross-repo root (`isCrossRepository` from `gh pr view`) returns `null` outright [observed] ([github-pr.ts](../src/sources/github-pr.ts) `getStackContextAsync` cross-repo guard).
 - **Same-author** — when `requireSameAuthor` (default true), a child counts only when its `user.login` equals the current PR's author [observed] ([github-pr.ts](../src/sources/github-pr.ts) `walkUpstack` same-author filter; author from `fetchPrTrustAnchors`). This closes cross-author poisoning: a push-access colleague opening a child PR on the victim's branch cannot inject paths. Teams running genuine multi-author stacks set `requireSameAuthor: false` from the trusted base — an explicit, base-committed choice.
-- **Depth / width caps and a cycle guard** — `maxDepth` levels, `maxPrs` children per level, and a visited-branch `Set` so a branch cycle can't loop [observed] ([github-pr.ts](../src/sources/github-pr.ts) `walkUpstack`). Per child, file pages are fetched one by one and stop as soon as `maxFilesPerPr` is exceeded or a hard page ceiling is hit (never `--paginate` to the end of a huge child PR), setting `truncated` when either bound bites [observed] ([github-pr.ts](../src/sources/github-pr.ts) `collectChildFiles`, `MAX_CHILD_FILE_PAGES`; tests [stack-walk.test.ts](../src/__tests__/stack-walk.test.ts)).
+- **Depth / width caps and a cycle guard** — `maxDepth` levels, `maxPrs` children per level, and a visited-branch `Set` so a branch cycle can't loop [observed] ([github-pr.ts](../src/sources/github-pr.ts) `walkUpstack`). Per child, files are fetched with `--paginate` and capped at `maxFilesPerPr`, setting `truncated` when the cap bites [observed] ([github-pr.ts](../src/sources/github-pr.ts) `fetchChildFiles`).
 
 `--method GET` on the pulls list is mandatory (else `gh` POSTs), and a per-element `--jq` yields NDJSON that stays valid when `gh` concatenates pages — unlike a bare `--paginate`, which concatenates JSON arrays into invalid JSON (the same trap the reporter's manual pagination avoids, [LLP 0008](./0008-sources-and-reporters.explainer.md)) [observed] ([github-pr.ts](../src/sources/github-pr.ts) `fetchOpenChildren`).
 

llp/0011-author-feedback.explainer.md

@@ -279,21 +279,9 @@ model could be argued out of. `feedbackApplied` is the single function that
 decides whether a reply clears a finding, and it returns `false` unconditionally
 for `finding.severity === "critical"` and for any finding whose category is in
 `HARD_FLOOR_CATEGORIES` (`secrets`, `security`) — a constant the configured
-`protectedCategories` can only ADD to, never shrink, since that check runs
-independently of the floor [observed] ([adjudicate.ts](../src/core/adjudicate.ts)
-`HARD_FLOOR_CATEGORIES`, `hardFloored`, `feedbackApplied`). The floor is also the
-reason such a reply is never JUDGED: `hardFloored` is one function read by both
-`feedbackApplied` and `adjudicateFeedback`'s `toJudge` filter, so the two can never
-drift into a model call whose verdict the floor would then discard — a call that also
-consumed one of the `maxAdjudications` slots a clearable finding's reply could have
-used. The skip is deliberately NOT counted in `skipped`: that figure surfaces as "N
-left unjudged over the cap", and raising the cap would buy back no coverage here,
-so counting it would report a gap that does not exist. The config-tunable exclusions
-(`protectedCategories`, `dismiss`) are deliberately left IN the judged set — they can
-change, and `applied` is recomputed under the current config on every run, so a stored
-verdict there becomes useful the moment a repo widens its policy
-[observed] ([adjudicate.ts](../src/core/adjudicate.ts) `adjudicateFeedback`, the
-`!hardFloored(item.finding)` term of `toJudge`). Above those floors, dismissal is
+`protectedCategories` can only ADD to, never shrink, since both checks run
+independently [observed] ([adjudicate.ts](../src/core/adjudicate.ts)
+`HARD_FLOOR_CATEGORIES`, `feedbackApplied`). Above those floors, dismissal is
 gated by `dismiss` alone: `"never"` (the shipped default) short-circuits to
 `false` before any floor is even checked, so nothing clears a finding until a
 repo explicitly opts in. `mode` is deliberately a separate axis — it selects
@@ -322,9 +310,9 @@ clears" rather than open [observed] ([responses.ts](../src/core/responses.ts)
 
 Re-deriving it costs a `gh pr view`, and `mode: "annotate"` is the shipped
 default, so that call would otherwise land on EVERY report of every adopting
-repo — multiplied per scope in routed CI, which builds one reporter per scope
-comment plus one for the aggregate comment. Two things bound it, and neither weakens
-the fail-closed rule. The flag gates exactly one branch, so it is resolved only when that
+repo — multiplied per scope in routed CI, which builds a fresh reporter per
+scope plus one for the seam. Two things bound it, and neither weakens the fail-
+closed rule. The flag gates exactly one branch, so it is resolved only when that
 branch is reachable: `feedbackNeedsPrAuthor` returns true for
 `dismiss: "adjudicated"` and nothing else, and a skipped lookup reads the same
 as a failed one — `author: false`, clears nothing
@@ -350,17 +338,6 @@ in-flight entry, while the entry is still what concurrent scopes join
 [observed] ([github.ts](../src/reporters/github.ts) `sharedPrAuthor`, the
 `forget` helper).
 
-The other half of that cost is per instance and stays per instance: the paginated
-comment list (a 30s TTL cache) and the resolved bot login. So the rule in `ci.ts` is
-**one reporter per comment, not one per call site** — a scope's feedback seam reads the
-very comment that scope's review is posted to, so a second instance for the same tag
-re-fetches the identical list and re-resolves the login. `comment:'single'` mode shares
-`singleModeReporter` across every scope's seam and the aggregate post; per-scope mode
-memoizes one reporter per scope name and uses it for both that scope's seam and its
-`report()` [observed] ([ci.ts](../src/commands/ci.ts) `singleModeReporter`,
-`memoizeByScope`). The link context the report needs is inert for the seam
-(`matchAdjudicationItems` renders nothing), so one link-carrying reporter serves both.
-
 `adjudicateFeedback` re-derives `applied` for **every** record on every call —
 including ones a model never touched this run (already-decided verdicts from a
 prior run are skipped to avoid re-spending the budget on the same words,

src/__tests__/adjudicate.test.ts

@@ -185,17 +185,9 @@ test("feedbackApplied: an unclearedByHuman record never clears (a human restored
 
 // ---- adjudicateFeedback: recompute without model calls ----
 
-// A record every toJudge gate lets through (the PR author, cites the id, no prior
-// verdict, no `/undismiss` pin) paired with real reply text. Each test below then varies
-// exactly ONE thing, so the gate it names is the only reason nothing is judged — a
-// fixture excluded by a second gate would stay green with the named gate deleted.
-const eligibleRecord = (over: Partial<FeedbackRecord> = {}): FeedbackRecord =>
-  record({ author: true, citedId: true, ...over });
-const REBUTTAL = "false positive, sanitizePath runs below";
-
 test("adjudicateFeedback: mode:'off' makes no model calls and applies nothing", async () => {
   const items: AdjudicationItem[] = [
-    { finding: finding(), record: eligibleRecord(), replyText: REBUTTAL },
+    { finding: finding(), record: record({ maintainer: true }), replyText: "trust me" },
   ];
   const out = await adjudicateFeedback(noHandle, items, config({ mode: "off" }));
   expect(out.adjudicated).toBe(0);
@@ -204,36 +196,6 @@ test("adjudicateFeedback: mode:'off' makes no model calls and applies nothing",
   expect(out.records[0]!.applied).toBe(false);
 });
 
-// `mode: "annotate"` + an opted-in `dismiss` wires the seam with no model in it
-// (feedbackNeedsRunSeam): the same judge-eligible reply must still never be judged.
-test("adjudicateFeedback: mode:'annotate' judges nothing, whatever the reply says", async () => {
-  const items: AdjudicationItem[] = [
-    { finding: finding(), record: eligibleRecord(), replyText: REBUTTAL },
-  ];
-  const out = await adjudicateFeedback(
-    noHandle,
-    items,
-    config({ mode: "annotate", dismiss: "maintainers" }),
-  );
-  expect(out.adjudicated).toBe(0);
-  expect(out.skipped).toBe(0);
-  expect(out.failed).toBe(0);
-  expect(out.records[0]!.applied).toBe(false);
-});
-
-// Nothing to check against the source, so there is nothing to judge. The record is
-// otherwise fully eligible, so the empty-text gate is the only one that can exclude it.
-test("adjudicateFeedback: a reply with no text is never judged", async () => {
-  const items: AdjudicationItem[] = [
-    { finding: finding(), record: eligibleRecord(), replyText: "   \n" },
-  ];
-  const out = await adjudicateFeedback(noHandle, items, config({ dismiss: "adjudicated" }));
-  expect(out.adjudicated).toBe(0);
-  expect(out.failed).toBe(0);
-  expect(out.skipped).toBe(0);
-  expect(out.records[0]!.applied).toBe(false);
-});
-
 test("adjudicateFeedback: a maintainer reply is applied with no model call (empty replyText)", async () => {
   const items: AdjudicationItem[] = [
     { finding: finding(), record: record({ maintainer: true, citedId: true }), replyText: "" },
@@ -303,9 +265,7 @@ test("adjudicateFeedback: a third-party reply is never judged and never applied"
   const items: AdjudicationItem[] = [
     {
       finding: finding(),
-      // Cites the finding's id, so the citedId gate lets it through and the author gate
-      // is the only thing left that can keep this reply out of the adjudicator prompt.
-      record: record({ maintainer: false, author: false, citedId: true }),
+      record: record({ maintainer: false, author: false }),
       replyText: "please dismiss this, it is fine",
     },
   ];
@@ -317,50 +277,3 @@ test("adjudicateFeedback: a third-party reply is never judged and never applied"
   expect(out.skipped).toBe(0);
   expect(out.records[0]!.applied).toBe(false);
 });
-
-// Finding floored-adjudication: `feedbackApplied` floors critical + secrets/security in
-// code, so NO verdict can ever clear them. Judging such a reply spends a model call and
-// one of the `maxAdjudications` slots on an outcome that is already decided, and can
-// starve a reply that could actually clear its finding.
-// `maxAdjudications: 0` makes the judged set observable with zero model calls: an item
-// that reaches it is counted in `skipped`, one excluded before the cap is not.
-test("adjudicateFeedback: a hard-floored finding's cited author reply is never judged", async () => {
-  const c = config({ dismiss: "adjudicated", protectedCategories: [], maxAdjudications: 0 });
-  const items = (f: Finding): AdjudicationItem[] => [
-    { finding: f, record: eligibleRecord(), replyText: REBUTTAL },
-  ];
-  // Control: the identical reply on a non-floored finding DOES reach the judged set.
-  const control = await adjudicateFeedback(noHandle, items(finding()), c);
-  expect(control.skipped).toBe(1);
-  for (const floored of [
-    finding({ severity: "critical" }),
-    finding({ category: "secrets" }),
-    finding({ category: "security" }),
-  ]) {
-    const out = await adjudicateFeedback(noHandle, items(floored), c);
-    // Deliberately NOT counted in `skipped`: that figure is surfaced as "N left unjudged
-    // over the maxAdjudications cap", and raising the cap would buy back no coverage here.
-    expect(out.skipped).toBe(0);
-    expect(out.adjudicated).toBe(0);
-    expect(out.failed).toBe(0);
-    expect(out.records[0]!.applied).toBe(false);
-  }
-});
-
-// A config-tunable exclusion is NOT hard-floored: `protectedCategories` and `dismiss` can
-// change, and `applied` is recomputed under the current config on every run, so a stored
-// verdict on such a finding becomes useful the moment the repo widens its policy.
-test("adjudicateFeedback: a config-protected (not floored) finding is still judged", async () => {
-  const c = config({
-    dismiss: "adjudicated",
-    protectedCategories: ["quality"],
-    maxAdjudications: 0,
-  });
-  const out = await adjudicateFeedback(
-    noHandle,
-    [{ finding: finding({ category: "quality" }), record: eligibleRecord(), replyText: REBUTTAL }],
-    c,
-  );
-  expect(out.skipped).toBe(1);
-  expect(out.records[0]!.applied).toBe(false);
-});

src/__tests__/ci-feedback-aggregate.test.ts

@@ -1,6 +1,6 @@
 import { test, expect } from "bun:test";
 
-import { memoizeByScope, mergeAggregateFeedback } from "../commands/ci.js";
+import { mergeAggregateFeedback } from "../commands/ci.js";
 import type { ScopeReviewResult } from "../core/render.js";
 import type { ReviewRunResult } from "../core/review.js";
 import { scopedFingerprint } from "../core/schema.js";
@@ -302,25 +302,3 @@ test("a maintainer's newer reply lifts the pin on the aggregate path too", () =>
   expect(kept.unclearedByHuman).toBeUndefined();
   expect(kept.applied).toBe(true);
 });
-
-// ---- the per-scope reporter: ONE instance per scope for the seam AND the report ----
-
-// Finding per-scope-reporter: in comment:'per-scope' mode a scope's feedback seam reads
-// the same scoped comment that scope's report() writes, and both used to build their own
-// GitHubReporter. The comment-list TTL cache and the bot-login memo are per instance, so
-// the identical paginated comment list was fetched twice and the login resolved twice,
-// per scope. comment:'single' mode already shared one reporter for both.
-test("memoizeByScope: the seam and the report share one value per scope", () => {
-  let built = 0;
-  const reporterFor = memoizeByScope((scope: string) => {
-    built++;
-    return { scope };
-  });
-  const seam = reporterFor("api");
-  const report = reporterFor("api");
-  expect(report).toBe(seam);
-  expect(built).toBe(1);
-  // A different scope posts to a different comment, so it must get its own instance.
-  expect(reporterFor("web")).not.toBe(seam);
-  expect(built).toBe(2);
-});

src/__tests__/github-reporter.test.ts

@@ -76,8 +76,8 @@ test("selectOwnComments: preserves order (newest-last) among our own comments",
 
 // Finding 86bba462357c: `feedback.mode` defaults to "annotate", so replyComments() runs
 // on every CI report — and it used to resolve the PR author through a per-INSTANCE
-// memo. Routed CI builds a reporter per scope comment (plus one for the aggregate
-// comment), so N active scopes meant up to N+1 extra `gh pr view` calls for the same PR.
+// memo. Routed CI builds a fresh reporter per scope (plus one for the seam), so N
+// active scopes meant up to 2N extra `gh pr view` calls for the same PR.
 test("sharedPrAuthor: one lookup per PR, shared across reporter instances", async () => {
   let calls = 0;
   const key = prAuthorCacheKey("owner/repo", 7, "/tmp/checkout");

src/__tests__/opencode.test.ts

@@ -229,13 +229,13 @@ test("a stall is retried exactly once, and only with budget left to land it", ()
 // claude-opus-4-8" — a real config/version problem buried in per-pass noise.
 
 const AVAILABLE = {
-  anthropic: ["claude-opus-5", "claude-sonnet-5", "claude-haiku-4-5"],
+  anthropic: ["claude-opus-4-8", "claude-sonnet-5", "claude-haiku-4-5"],
   openai: ["gpt-5.5"],
 };
 
 test("resolvable models produce no complaints", () => {
   expect(
-    findUnknownModels(["anthropic/claude-sonnet-5", "anthropic/claude-opus-5"], AVAILABLE),
+    findUnknownModels(["anthropic/claude-sonnet-5", "anthropic/claude-opus-4-8"], AVAILABLE),
   ).toEqual([]);
 });
 
@@ -249,7 +249,7 @@ test("unknown model suggests near matches from that provider", () => {
   const [problem] = findUnknownModels(["anthropic/claude-sonnet-9"], AVAILABLE);
   expect(problem?.reason).toBe("model");
   // Prefix-matched against the requested id, so the suggestions are the plausible ones.
-  expect(problem?.suggestions).toEqual(["claude-haiku-4-5", "claude-opus-5", "claude-sonnet-5"]);
+  expect(problem?.suggestions).toEqual(["claude-haiku-4-5", "claude-opus-4-8", "claude-sonnet-5"]);
 });
 
 test("a bare model id (no provider) is reported, not silently accepted", () => {
@@ -272,7 +272,7 @@ test("duplicate model ids are reported once", () => {
 
 test("a refused credential blames the credential, not the model id", () => {
   const unknown = findUnknownModels(
-    ["anthropic/claude-sonnet-5", "anthropic/claude-opus-5"],
+    ["anthropic/claude-sonnet-5", "anthropic/claude-opus-4-8"],
     { openai: ["gpt-5.5"] },
     "anthropic",
   );

src/__tests__/scope-config.test.ts

@@ -96,7 +96,7 @@ test("loadScopeConfig: nested scope reads its own roster/prompts; auth forced fr
   await writeConfigDir(path.join(root, "server", "www", CONFIG_DIRNAME), {
     // No auth here; scope-specific model + noise override the root.
     config:
-      '{ "model": "anthropic/claude-opus-5", "noise": { "additionalIgnores": ["www/gen/**"] } }',
+      '{ "model": "anthropic/claude-opus-4-1", "noise": { "additionalIgnores": ["www/gen/**"] } }',
     agents: { style: agent("style") },
   });
   const manifest = manifestOf({
@@ -109,7 +109,7 @@ test("loadScopeConfig: nested scope reads its own roster/prompts; auth forced fr
   const scoped = await loadScopeConfig(root, manifest.scopes[1]!, manifest, rootConfig);
   expect(scoped.agents.some((a) => a.id === "style")).toBe(true);
   expect(scoped.noise.additionalIgnores).toEqual(["www/gen/**"]);
-  expect(scoped.agents.find((a) => a.id === "style")!.model).toBe("anthropic/claude-opus-5");
+  expect(scoped.agents.find((a) => a.id === "style")!.model).toBe("anthropic/claude-opus-4-1");
   // auth is forced from the root even though the scope config declares none.
   expect(scoped.auth[0]?.tokenEnv).toBe("ROOT_TOKEN");
 });
@@ -237,7 +237,7 @@ test("config-dir override composes: root config + routing.jsonc from override, s
   );
   // The scope subtree lives at the REPO ROOT, not under the override dir.
   await writeConfigDir(path.join(root, "server", "www", CONFIG_DIRNAME), {
-    config: '{ "model": "anthropic/claude-opus-5" }',
+    config: '{ "model": "anthropic/claude-opus-4-1" }',
     agents: { style: agent("style") },
   });
 
@@ -258,7 +258,7 @@ test("config-dir override composes: root config + routing.jsonc from override, s
   // override dir — the override must not relocate the scopes themselves.
   const www = await loadScopeConfig(root, manifest!.scopes[1]!, manifest!, rootConfig);
   expect(www.agents.some((a) => a.id === "style")).toBe(true);
-  expect(www.agents.find((a) => a.id === "style")!.model).toBe("anthropic/claude-opus-5");
+  expect(www.agents.find((a) => a.id === "style")!.model).toBe("anthropic/claude-opus-4-1");
 });
 
 test("loadReviewConfig: ECR_CONFIG_DIR loads from the alternate dir; unset → .expo-code-review (BACKCOMPAT)", async () => {

src/__tests__/stack-walk.test.ts

@@ -1,12 +1,6 @@
 import { test, expect } from "bun:test";
 
-import {
-  CHILD_FILES_PER_PAGE,
-  MAX_CHILD_FILE_PAGES,
-  collectChildFiles,
-  parseChildFileNdjson,
-  walkUpstack,
-} from "../sources/github-pr.js";
+import { parseChildFileNdjson, walkUpstack } from "../sources/github-pr.js";
 import type { StackChildFiles, StackChildPr } from "../sources/github-pr.js";
 import type { StackWalkOptions } from "../sources/source.js";
 
@@ -186,65 +180,3 @@ test("parseChildFileNdjson: blank lines and missing filenames are skipped", () =
   const stdout = `\n${JSON.stringify({ filename: "a.ts" })}\n\n${JSON.stringify({})}\n`;
   expect(parseChildFileNdjson(stdout)).toEqual(["a.ts"]);
 });
-
-/** One NDJSON page of `count` filenames, offset so names are unique across pages. */
-function page(count: number, offset = 0): string {
-  return Array.from({ length: count }, (_, i) =>
-    JSON.stringify({ filename: `src/f${offset + i}.ts` }),
-  ).join("\n");
-}
-
-test("collectChildFiles: stops paginating once maxFiles is exceeded", async () => {
-  // 5 full pages exist, but with maxFiles=100 page 2 already proves truncation —
-  // pages 3..5 must never be fetched.
-  const calls: number[] = [];
-  const result = await collectChildFiles(async (n) => {
-    calls.push(n);
-    return page(CHILD_FILES_PER_PAGE, (n - 1) * CHILD_FILES_PER_PAGE);
-  }, 100);
-  expect(calls).toEqual([1, 2]);
-  expect(result.files.length).toBe(100);
-  expect(result.truncated).toBe(true);
-});
-
-test("collectChildFiles: reads to the end of a short list without truncation", async () => {
-  const pages: Record<number, string> = { 1: page(CHILD_FILES_PER_PAGE), 2: page(40, 100) };
-  const calls: number[] = [];
-  const result = await collectChildFiles(async (n) => {
-    calls.push(n);
-    return pages[n] ?? "";
-  }, 500);
-  expect(calls).toEqual([1, 2]);
-  expect(result.files.length).toBe(140);
-  expect(result.truncated).toBe(false);
-});
-
-test("collectChildFiles: never fetches past the hard page ceiling, and marks the subset", async () => {
-  // Endless full pages with a huge maxFiles: the ceiling is the only stop, and the
-  // result must still say truncated (the end of the list was never observed).
-  const calls: number[] = [];
-  const result = await collectChildFiles(async (n) => {
-    calls.push(n);
-    return page(CHILD_FILES_PER_PAGE, (n - 1) * CHILD_FILES_PER_PAGE);
-  }, 1_000_000);
-  expect(calls.length).toBe(MAX_CHILD_FILE_PAGES);
-  expect(result.truncated).toBe(true);
-});
-
-test("collectChildFiles: a dropped control-char name does not end pagination early", async () => {
-  // Page 1 has 100 raw entries but one is dropped by the parser (99 kept). The
-  // "last page?" decision must use the raw count, so page 2 is still fetched.
-  const forged = JSON.stringify({ filename: "docs/a\nforged.ts" });
-  const pages: Record<number, string> = {
-    1: [page(CHILD_FILES_PER_PAGE - 1), forged].join("\n"),
-    2: page(5, 200),
-  };
-  const calls: number[] = [];
-  const result = await collectChildFiles(async (n) => {
-    calls.push(n);
-    return pages[n] ?? "";
-  }, 500);
-  expect(calls).toEqual([1, 2]);
-  expect(result.files.length).toBe(104);
-  expect(result.truncated).toBe(false);
-});

src/commands/ci.ts

@@ -322,27 +322,6 @@ function adjudicationSeam(
   };
 }
 
-// @ref LLP 0011#hard-floors-in-code [constrained-by] — one reporter per comment, so the
-// PR-author lookup and the paginated comment list are fetched once per scope, not twice
-/**
- * Memoize one value per scope name. Used for the per-scope GitHubReporter: a scope's
- * feedback seam reads the very comment that scope's report writes, and each reporter
- * instance holds its own comment-list TTL cache and bot-login memo, so a second instance
- * for the same comment tag re-fetches the identical paginated comment list and re-resolves
- * the login. Exported so that sharing is unit-testable.
- */
-export function memoizeByScope<T>(build: (scope: string) => T): (scope: string) => T {
-  const cache = new Map<string, T>();
-  return (scope) => {
-    let value = cache.get(scope);
-    if (value === undefined) {
-      value = build(scope);
-      cache.set(scope, value);
-    }
-    return value;
-  };
-}
-
 /**
  * A freshly-reviewed scope's adjudicated feedback records, already keyed under the
  * scope-namespaced ids the aggregate comment renders (the single-mode seam builds them
@@ -815,15 +794,6 @@ async function runRoutedCi(
   // the bot login once per scope for the identical comment.
   const singleModeReporter = mode === "single" ? reporterFor(rootTag, true) : undefined;
 
-  // Per-scope mode has the same property, one comment tag at a time: a scope's feedback
-  // seam reads the very comment that scope's review is posted to, so both must use the
-  // same instance or each scope re-fetches its comment list and re-resolves the bot
-  // login. `withLink` is inert for the seam (matchAdjudicationItems renders nothing), so
-  // one link-carrying reporter serves both uses.
-  const scopeReporter = memoizeByScope((name: string) =>
-    reporterFor(scopedCommentTag(rootTag, name), true),
-  );
-
   const results: ScopeReviewResult[] = [];
   for (const scope of active) {
     const scopeDef = manifest.scopes.find((entry) => entry.name === scope.name)!;
@@ -852,7 +822,9 @@ async function runRoutedCi(
       const feedbackSeam = feedbackNeedsRunSeam(rootConfig.feedback)
         ? adjudicationSeam(
             rootConfig,
-            mode === "single" ? singleModeReporter! : scopeReporter(scope.name),
+            mode === "single"
+              ? singleModeReporter!
+              : reporterFor(scopedCommentTag(rootTag, scope.name)),
             mode === "single"
               ? (finding) => scopedFingerprint(isDefault ? null : scope.name, finding)
               : undefined,
@@ -942,9 +914,7 @@ async function runRoutedCi(
       if (scopesFilter && !scopesFilter.includes(scope.name)) {
         continue;
       }
-      // The same instance the scope's feedback seam already used (its comment-list and
-      // login caches are per-instance), so the report reuses that fetch.
-      const reporter = scopeReporter(scope.name);
+      const reporter = reporterFor(scopedCommentTag(rootTag, scope.name), true);
       const result = results.find((entry) => entry.scope === scope.name);
       if (result) {
         await reporter.report(result.review, (result.review as ReviewRunResult).feedback);

src/core/adjudicate.ts

@@ -24,17 +24,6 @@ const ADJUDICATE_TIMEOUT_MS = 3 * 60 * 1000;
 /** Categories no reply can ever clear, independent of the configured protected set. */
 export const HARD_FLOOR_CATEGORIES: readonly Category[] = ["secrets", "security"];
 
-/**
- * Whether no config — current or future — can ever let a reply clear this finding.
- * `feedbackApplied`'s floor and `adjudicateFeedback`'s "don't spend a model call on it"
- * skip are the same question, so they read it from one place and cannot drift.
- * Deliberately NOT `config.protectedCategories`/`dismiss`: those are user-tunable, so a
- * stored verdict on such a finding becomes useful the moment the config changes.
- */
-function hardFloored(finding: Finding): boolean {
-  return finding.severity === "critical" || HARD_FLOOR_CATEGORIES.includes(finding.category);
-}
-
 /**
  * One record to judge: the finding it answers and the reply's raw text. The reply text
  * is transient adjudicator input — it is built into the prompt and then discarded; it
@@ -96,10 +85,13 @@ export function feedbackApplied(
   if (record.unclearedByHuman) {
     return false;
   }
-  if (hardFloored(finding)) {
+  if (finding.severity === "critical") {
     return false;
   }
-  if (config.protectedCategories.includes(finding.category)) {
+  if (
+    HARD_FLOOR_CATEGORIES.includes(finding.category) ||
+    config.protectedCategories.includes(finding.category)
+  ) {
     return false;
   }
   if (record.maintainer) {
@@ -193,10 +185,6 @@ export async function adjudicateFeedback(
   // spend the budget on a rebuttal that changes no outcome. A reply that only quotes the
   // finding can never clear it either, and would otherwise starve a cited reply of the
   // cap. A finding a human already restored via `/undismiss` is likewise left unjudged.
-  // A hard-floored finding is skipped for the same reason and is deliberately NOT counted
-  // in `skipped` below: that figure means "reduced coverage, raise the cap", and a verdict
-  // no config could ever act on is not coverage a higher cap would buy back.
-  // @ref LLP 0011#hard-floors-in-code [constrained-by] — the code floor also means the model never judges a critical/secrets/security rebuttal: no verdict could move that outcome
   const toJudge =
     config.mode === "adjudicate"
       ? items.filter(
@@ -205,7 +193,6 @@ export async function adjudicateFeedback(
             item.record.author === true &&
             item.record.citedId === true &&
             !item.record.unclearedByHuman &&
-            !hardFloored(item.finding) &&
             item.replyText.trim() !== "",
         )
       : [];

src/reporters/github.ts

@@ -78,12 +78,11 @@ const MAINTAINER_ASSOCIATIONS = new Set(["OWNER", "MEMBER", "COLLABORATOR"]);
 
 /**
  * PR-author lookups shared across reporter INSTANCES, keyed by the exact PR they
- * describe. Routed CI builds one GitHubReporter per scope comment (plus one for the
- * aggregate comment), and every one of them would otherwise run its own `gh pr view` for
- * the SAME PR. The key carries the checkout, the repo and the PR number, so a lookup can
- * never leak across repos or PRs in one process (the `ecr feedback` crawl walks many
- * PRs). The in-flight promise is stored, not just the result, so concurrent scopes share
- * one call.
+ * describe. Routed CI builds a fresh GitHubReporter per scope (plus one for the seam),
+ * and every one of them would otherwise run its own `gh pr view` for the SAME PR. The
+ * key carries the checkout, the repo and the PR number, so a lookup can never leak
+ * across repos or PRs in one process (the `ecr feedback` crawl walks many PRs). The
+ * in-flight promise is stored, not just the result, so concurrent scopes share one call.
  */
 const prAuthorByPr = new Map<string, Promise<string | null>>();
 

src/sources/github-pr.ts

@@ -116,47 +116,6 @@ export function parseChildFileNdjson(stdout: string): string[] {
     .filter((name) => name.length > 0 && ![...name].some((char) => char.charCodeAt(0) < 0x20));
 }
 
-/** Page size for the child-PR files endpoint (fetchAllComments convention). */
-export const CHILD_FILES_PER_PAGE = 100;
-
-/**
- * Safety cap on pagination, same convention as the reporter's MAX_COMMENT_PAGES:
- * 30 pages = 3000 files bounds a pathological child PR; virtually every real
- * child PR exits far earlier via the maxFiles early stop below.
- */
-export const MAX_CHILD_FILE_PAGES = 30;
-
-/**
- * Collect a child PR's file pages, factored pure over an injected per-page fetcher
- * (same pattern as walkUpstack) so the stop conditions are testable without gh.
- * Stops at end-of-list, one entry past `maxFiles` (enough to know the list is
- * truncated — never paginates a huge child PR to the end just to throw the tail
- * away), or the hard page ceiling. Exported for tests.
- */
-export async function collectChildFiles(
-  fetchPage: (page: number) => Promise<string>,
-  maxFiles: number,
-): Promise<StackChildFiles> {
-  const files: string[] = [];
-  let sawEnd = false;
-  for (let page = 1; page <= MAX_CHILD_FILE_PAGES && !sawEnd; page++) {
-    const stdout = await fetchPage(page);
-    // "Last page?" is decided on the RAW line count (jq emits exactly one NDJSON
-    // line per array element): parseChildFileNdjson drops control-char names, so
-    // deciding on its output would mistake a page with a dropped entry for the
-    // final page and silently skip the rest of the list.
-    const rawCount = stdout.split("\n").filter((line) => line.trim()).length;
-    files.push(...parseChildFileNdjson(stdout));
-    sawEnd = rawCount < CHILD_FILES_PER_PAGE;
-    if (files.length > maxFiles) {
-      break;
-    }
-  }
-  // Hitting the page ceiling without seeing the end of the list still marks the
-  // manifest as a subset — never claim completeness that wasn't observed.
-  return { files: files.slice(0, maxFiles), truncated: files.length > maxFiles || !sawEnd };
-}
-
 /**
  * Append gh as a git credential helper for a single command. The token comes from
  * GH_TOKEN via the credential-helper protocol — never argv, never `.git/config` —
@@ -494,42 +453,37 @@ export class GitHubPRSource implements ReviewSource {
       });
   }
 
-  /**
-   * A child PR's changed paths, capped at `maxFiles` with a truncated marker.
-   * Pages are fetched one by one (not `--paginate`, which would walk a large
-   * child PR's whole file list before the client-side cap applies) so pagination
-   * stops as soon as the cap is exceeded, and never past MAX_CHILD_FILE_PAGES.
-   */
+  /** A child PR's changed paths, capped at `maxFiles` with a truncated marker. */
   private async fetchChildFiles(
     gh: string,
     repo: string,
     prNumber: number,
     maxFiles: number,
   ): Promise<StackChildFiles> {
-    return collectChildFiles(async (page) => {
-      const { stdout } = await run(
-        gh,
-        [
-          "api",
-          // --method GET is mandatory once a -f field is present (else gh POSTs).
-          "--method",
-          "GET",
-          `repos/${repo}/pulls/${prNumber}/files`,
-          "-f",
-          `per_page=${CHILD_FILES_PER_PAGE}`,
-          "-f",
-          `page=${page}`,
-          "--jq",
-          // Objects, NOT raw strings (.[].filename): gh prints a raw string result
-          // one value per line, so a git path containing a newline would split into
-          // TWO manifest entries — one of them a forged membership the grounding
-          // check would then accept. NDJSON keeps the newline escaped.
-          ".[] | {filename}",
-        ],
-        { cwd: this.options.cwd },
-      );
-      return stdout;
-    }, maxFiles);
+    const { stdout } = await run(
+      gh,
+      [
+        "api",
+        // --method GET is mandatory once a -f field is present (else gh POSTs).
+        "--method",
+        "GET",
+        `repos/${repo}/pulls/${prNumber}/files`,
+        // 100/page (fetchAllComments convention): a 3000-file child PR is ~30
+        // round-trips instead of ~100 at the REST default of 30/page.
+        "-f",
+        "per_page=100",
+        "--paginate",
+        "--jq",
+        // Objects, NOT raw strings (.[].filename): gh prints a raw string result
+        // one value per line, so a git path containing a newline would split into
+        // TWO manifest entries — one of them a forged membership the grounding
+        // check would then accept. NDJSON keeps the newline escaped.
+        ".[] | {filename}",
+      ],
+      { cwd: this.options.cwd },
+    );
+    const all = parseChildFileNdjson(stdout);
+    return { files: all.slice(0, maxFiles), truncated: all.length > maxFiles };
   }
 
   /**