Close out Cut B: pin the walked candidates and source the last cell (#456) - #507
Conversation
…last cell (#456) Sourcing for the `pre_1_0` corpus is finished: all 60 slots are `pinned`, and nothing in the inventory needs mining before the Cut C calibration round. Three things stood in the way, and one sweep (`benchmark/miner/results/2026-W36-closeout`, 45 rows over three repositories) closes all of them. - **The last gap.** `langchain_crewai × insufficient_evidence` is claimed by `bytedance/deer-flow#4868`, following Session B's lead to mine an *application* that calls `MultiServerMCPClient` at agent construction rather than the adapter library. Its tools are assembled at run time from an out-of-tree extensions config, and the change makes which credential a tool call carries depend on the run-time user and a `$ENV_VAR` map. - **Three walk candidates, pinned.** `github-mcp-server#3020` and `#3076` and `grafana/mcp-grafana#1080` were carried with abbreviated or absent SHAs. `#3076` is why the convention matters: its walk note's head `5ea9a0e8…` is `refs/pull/3076/head`, which after a squash merge is not reachable from the default branch at all. - **The guard the pinning section names was not in the tree.** It was added in Cut A and removed by Cut A's own review commit, which added the sentence citing it. Restored and generalized: every external pin is re-read from the sweep that resolved it, a pin no sweep corroborates is refused, and a subject a sweep did resolve may not sit `unpinned`. Two smaller things fall out. Filling the last gap makes the "a gap that names a PR plans the origin that PR can supply" tripwire vacuous, so it is re-pointed at the reserve — which states the same origin-beside-state pair, and is where the next candidate comes from. And the Cut B finding that the SDK repositories' example trees are "no longer picked up by cold-start `init`" is corrected: reproducing it at the recorded pin gives `refused_unresolved_scope`, the monorepo behavior working as designed. No engine change: nothing under `src/` moves, per the Cut B rule that a fix made in response to a candidate is what turns it into tuning material. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…456) Review of the close-out found four things; two are guards that failed open on the shape they exist for, and two are claims stated more confidently than the evidence supports. **A sweep file that contradicts itself was read as agreement.** `_swept_pins` keyed recordings by sweep file, so one file holding a subject twice with different SHAs kept whichever row came last: a corrupt duplicate ordered *before* the good row disappeared. Reproduced against the pre-fix keying — the disagreement check and the pin comparison both passed. Keyed by the pins instead, with the sweeps that recorded them as the value, so any two recordings that disagree fail whether they sit in one file or two. **A reserve row whose state can supply no origin was skipped rather than failed.** `_reserve_claims` dropped every row whose `State` was outside `STATE_ORIGINS`, which includes `open` — so `adk-samples#1745`, the candidate that taught this project an open PR is not history, could have been reserved as `real_history` and passed. A row that states an origin now has to state a state that can supply one. **Two claims corrected.** deer-flow#4868 merged five days before the latest-40 window's oldest PR, not six weeks: that repository merges forty PRs in six days, where crewAI-examples' forty reach back eighteen months, so `--limit 40` is not a time window and a busy repository's silence under it means very little. And `init`'s refusal reports the agent-defining projects it found while warning the list is incomplete, so the narrative no longer states the count as an enumeration. Guard sweep re-run: eight perturbations, all fail closed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…rd refuses (#456) Second review pass, three prose corrections and one addition. - "The three walked MCP servers all read `tools_scanned=0`" counted candidates as servers: three pinned candidates, two repositories. The walks covered four candidates across three repositories, so the sentence contradicted the inventory it summarizes. - The close-out run note now says *why* it stays off the `*-mined` glob in its own terms rather than by analogy: deer-flow's trigger-skip rate is 14 of 40, because it is an agent application chosen for one cell, not the unselected sample the noise bound measures. - The pinning section now states what the restored guard does to a candidate that cannot be mined at all — a private design-partner repository fails it rather than passing on a hand-written pin, and accepting one anyway is an owner's decision rather than a silent exception. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
pengfei-threemoonslab
left a comment
There was a problem hiding this comment.
Expert review completed on 8de35e2. The current 45-row artifacts match exactly, the four newly used upstream pins follow the merge-commit/first-parent convention, the targeted inventory and miner-corpus suites pass, Ruff/compileall/diff-check pass, GitHub CI is green, and the local Agents Shipgate verifier reports complete/passed.
I found two durability gaps in the corpus guards plus three documentation/data consistency issues below. Because this GitHub identity owns the PR, GitHub only permits a COMMENT review; substantively, the P2 items should be addressed before merge.
| """ | ||
|
|
||
| pins: dict[str, dict[tuple[str, str], set[str]]] = {} | ||
| for source in sorted(MINER_RESULTS.glob("*.csv")): |
There was a problem hiding this comment.
[P2] Bind every accepted results CSV to its JSONL sibling. This loop now treats all CSVs as pin authority, but test_miner_corpus.py validates well-formedness and CSV/JSONL parity only for *-mined.jsonl. The new 2026-W36-closeout pair (and W36-cutb) is intentionally outside that glob, so a half-regeneration can leave this CSV stale while JSONL changes and these stale pins still certify the inventory; corruption in most other rows also bypasses CI. Add a separate all-results integrity enumeration while keeping only the KPI aggregation on *-mined, or validate each sibling JSONL before accepting pins here.
There was a problem hiding this comment.
Done in 8ff68ef. Split the enumeration in tests/test_miner_corpus.py: _all_run_jsonl_files() (every *.jsonl) now drives well-formedness, CSV/JSONL parity and the LF check, and _mined_jsonl_files() keeps only the KPI aggregation, since the noise bound is meaningful over unselected sweeps alone. All seven committed runs pass unchanged, so the four that were outside the glob — -reeval, -cutb, -closeout, constructed — are load-bearing from this commit.
Two guards keep the split from decaying. test_at_least_one_committed_run_exists now also asserts the mined set is a strict subset, so the two enumerations cannot silently converge; and test_every_committed_results_csv_has_a_jsonl_to_be_checked_against fails on a results CSV with no JSONL sibling, which is the one way a file could still be pin authority while escaping the enumeration entirely. Both perturbed: a one-character edit to the closeout CSV fails test_csv_and_jsonl_agree[2026-W36-closeout.jsonl], and an orphan CSV fails the sibling guard.
| entry.get("head_sha"), | ||
| ) | ||
| if url and base and head: | ||
| recorded = pins.setdefault(_candidate_ref_for(url), {}) |
There was a problem hiding this comment.
[P2] Commit the fail-closed perturbations as regression tests. All checked-in inputs are valid, so the suite still passes if this helper regresses to the earlier per-sweep last-write-wins implementation, or if _reserve_claims regresses to skipping an open row that states real_history—the exact two holes fixed on this branch. Refactor the parsers to accept test paths/text and add temporary fixtures for a contradictory duplicate in one sweep (both row orders) and open-plus-origin / misspelled reserve claims. The manual perturbation sweep in the PR description will not protect later edits.
There was a problem hiding this comment.
Done in 8ff68ef, and you are right that the manual sweep protected nothing — I reverted each implementation to confirm it.
Both parsers now take their input (_swept_pins(sources=...), _reserve_claims(lines=...)), and the row-versus-sweep rule moved out of the test body into _pin_complaints(rows, swept) so a fixture goes through the same code the committed inventory does; the guard is now one assertion that the complaint list is empty. _unsuppliable_origins(claims) does the same for the reserve rule.
Committed perturbations: a contradictory duplicate inside one sweep parametrized over both row orders (only bad-first ever failed, which is exactly why the bug survived), an uncorroborated pin, a resolved subject left unpinned, an open PR reserved with a real origin, and a misspelled reserve origin. Verified by regression rather than by construction — reverting _swept_pins to per-sweep last-write-wins fails both duplicate cases, and reverting _reserve_claims to continue-on-unknown-state fails both reserve cases.
| mcp_openapi_declared_binding.insufficient_evidence.2,mcp_openapi_declared_binding,insufficient_evidence,synthetic,none,either,pinned,benchmark/safety-qualification/constructed/mcp_export_adds_undeclared_tool,,,constructed_design,benchmark/safety-qualification/constructed/mcp_export_adds_undeclared_tool/CASE.md,,"Adds a fourth tool, zendesk.add_ticket_comment, to the committed MCP export while the manifest's complete root declaration still lists three." | ||
| mcp_openapi_declared_binding.blocked.1,mcp_openapi_declared_binding,blocked,design_partner,engine_tests;maintainer_walk;benchmark_scored;miner_label,tuning_only,pinned,github.com/stripe/ai#232,5af4bcd15813cbcbd91baceeb5ec79cf975035f1,cd8cee575064db6ae00cee9984f976dd5055f9c2,miner_label,benchmark/miner/results/2026-W24-mined.labels.csv,,"The design-partner pilot case: removes the client-side toolkit's action and permission least-privilege bounds entirely, delegating all tool authority to a server-side key through an async factory." | ||
| mcp_openapi_declared_binding.blocked.2,mcp_openapi_declared_binding,blocked,real_history,engine_tests;maintainer_walk,tuning_only,unpinned,github.com/github/github-mcp-server#3076,,,diff_substance,benchmark/safety-qualification/strata-inventory.md#candidate-register,,Adds a confirmed repository-deletion tool. The walk recorded base bfb59bb7.. / head 5ea9a0e8.. as abbreviations; resolve both to full SHAs under the pinning convention below. | ||
| mcp_openapi_declared_binding.blocked.2,mcp_openapi_declared_binding,blocked,real_history,engine_tests;maintainer_walk;benchmark_scored,tuning_only,pinned,github.com/github/github-mcp-server#3076,bfb59bb7862f54362d832f083e2255a263f2b814,8ec62491c61715e190bf9a01aa66eed8f5bc0240,diff_substance,benchmark/safety-qualification/strata-inventory.md#candidate-register,,Adds a confirmed repository-deletion tool. The walk recorded base bfb59bb7.. / head 5ea9a0e8.. as abbreviations; resolve both to full SHAs under the pinning convention below. |
There was a problem hiding this comment.
[P3] Remove the now-completed instruction from this pinned row. The full base and merge-commit pins are present, but the note still tells the reader to resolve both SHAs and foregrounds the discarded 5ea9a0e8 PR-head abbreviation. State that the close-out resolved the usable head to 8ec62491 instead.
There was a problem hiding this comment.
Done in 8ff68ef. The note now reads: "Pinned by the close-out sweep at the merge commit 8ec62491 and its first parent bfb59bb7; the walk note also carried 5ea9a0e8, which is the PR branch head and not reachable from the default branch." The instruction is gone and the discarded abbreviation is demoted to the reason the resolution needed stating, which matches the register entry for the same candidate.
| | [`2026-W26-mined.csv`](results/2026-W26-mined.csv) | 2026-06-16 | stripe/agent-toolkit → **stripe/ai** (see note), block/goose, pydantic/pydantic-ai | 120 (latest 40 merged PRs each) | Deepen run over agent **apps/toolkits**. First run with `tools_scanned` captured (#223); decided rows are cold-start `head_decision=review_required` but `verify`-effective `insufficient_evidence`. Schema v0.2. Findings below. | | ||
| | [`2026-W27-reeval.csv`](results/2026-W27-reeval.csv) | 2026-07-08 | the 19 labeled PRs (stripe/ai, openai/openai-agents-python, crewAIInc/crewAI-examples, google/adk-samples, aaif-goose/goose — formerly block/goose) | 19 (re-eval at fixed SHAs, not a fresh mine) | **v0.15.0 delta on the labeled corpus.** Same PRs / same base→head SHAs as W24–W26, re-run on the released engine. Clears the 4 scan crashes; both `must_block` move abstain→review but `blocked_recall` stays 0.0. Off the `*-mined` glob by design. Findings below. | | ||
| | [`2026-W36-cutb.csv`](results/2026-W36-cutb.csv) | 2026-09-02 | n8n-io/n8n, n8n-io/self-hosted-ai-starter-kit, Zie619/n8n-workflows, enescingoz/awesome-n8n-templates, modelcontextprotocol/servers, microsoft/playwright-mcp, cloudflare/mcp-server-cloudflare, supabase-community/supabase-mcp, Azure/azure-mcp, hashicorp/terraform-mcp-server, elastic/mcp-server-elasticsearch, redis/mcp-redis, openai/openai-agents-python, google/adk-samples, google/adk-python, langchain-ai/langgraph, langchain-ai/langchain-mcp-adapters, langchain-ai/deepagents, crewAIInc/crewAI-examples, aaif-goose/goose, pydantic/pydantic-ai | 912 (latest 40 merged per repo, plus `--state closed` on openai-agents-python and adk-samples, `--state reverted` on the same plus goose and pydantic-ai, and `--pr` named candidates) | **The Cut B sourcing sweep for [#456](https://github.com/ThreeMoonsLab/agents-shipgate/issues/456), and the first post-#403 run.** Every n8n repository this project had never mined, eight unwalked MCP servers, and the rejected vein (closed-unmerged + reverted PRs, new in this run). Schema v0.2. **Labeled** in `2026-W36-cutb.labels.csv` — one session's Cut B cell-targeting labels from the PR diffs, *not adjudicated*; corpus labels come only from the Amendment 1 raters. Off the `*-mined` glob by design (see note). Findings below. | | ||
| | [`2026-W36-closeout.csv`](results/2026-W36-closeout.csv) | 2026-09-02 | github/github-mcp-server, grafana/mcp-grafana, bytedance/deer-flow | 45 (three `--pr` named walk candidates, the latest 40 merged on deer-flow, and two `--pr` named deer-flow candidates) | **The Cut B close-out for [#456](https://github.com/ThreeMoonsLab/agents-shipgate/issues/456).** Resolves the pins for the three walked MCP servers the inventory carried unpinned, and mines the LangChain application repository that closes its last cell. Schema v0.2. **Labeled** in `2026-W36-closeout.labels.csv` — one row, the claimed candidate, cell-targeting and *not adjudicated* like every other sourcing label here. Off the `*-mined` glob by design: every row is either a named PR or a repository chosen for one cell, and its own trigger-skip rate is 14 of 40 (0.35) on the one repository swept by window — an agent application, not the unselected sample the noise bound measures. Findings below. | |
There was a problem hiding this comment.
[P3] This still counts three candidates as three servers. PRs 3020 and 3076 are two changes to the same github-mcp-server; the close-out covers three walked candidates across two MCP-server repositories. This is the same overcount the later findings prose already corrects, so use “three walked candidates” here as well.
There was a problem hiding this comment.
Done in 8ff68ef — the run-table row now says "the three walked candidates the inventory carried unpinned (two MCP-server repositories)". Same overcount, missed because I fixed the findings prose below and not the row above it.
| |---|---| | ||
| | Slots with a candidate | 59 of 60 | | ||
| | Gaps to mine or construct | 1 | | ||
| | Slots with a candidate | 60 of 60 | |
There was a problem hiding this comment.
[P3] Reconcile the newly complete table with the surrounding current-state narrative. Lines 327–330 still say the plan “reaches 32” by committing to mine 18 further qualifying candidates, and lines 556–558 still put “Cut B mines the gaps” in the future, even though this table now says 33 sourced / 0 left and the added close-out says sourcing is finished. Update those counts and tense so the reviewer handoff has one current status.
There was a problem hiding this comment.
Done in 8ff68ef, all three places. The origin-floor paragraph is now past tense and stops restating a count the table above owns ("clearing that floor took 18 further qualifying candidates, more than half of it, which Cut B and the close-out mined. The counts the plan holds now are in the tables above, which are recomputed from the CSV"). The order-of-work line says Cut B and the close-out are both done and names the calibration round as next, and the paragraph under it no longer repeats the status. I also found a third instance you did not flag: the file opened with "so Cut B mines the empty cells", which now reads "could mine" and carries the current status in the same paragraph. One status, at the top, restated nowhere.
…perturbations (#456) Addresses the five review findings on this PR. **[P2] Every results CSV is pin authority, so every results run is now integrity-checked.** `test_miner_corpus.py` validated well-formedness, CSV/JSONL parity and LF endings over `*-mined.jsonl` only, while `_swept_pins` reads *every* committed CSV as the authority for a corpus candidate's pins — so a half-regenerated `-cutb`, `-closeout`, `-reeval` or `constructed` pair could leave stale pins certifying the inventory with nothing in CI to notice. The three integrity guards move to an all-runs enumeration; only the KPI aggregation stays on `*-mined`, because the trigger-skip noise bound is only meaningful over unselected sweeps. Two guards keep the split honest: the enumerations may not converge, and a results CSV with no JSONL sibling — which would escape the enumeration entirely while still supplying pins — fails. **[P2] The two holes fixed on this branch now have regression tests.** Every committed input is valid, so the suite passed either way: reverting `_swept_pins` to per-sweep last-write-wins, or `_reserve_claims` to skipping a state outside `STATE_ORIGINS`, went undetected. Both parsers now take their input, the row-versus-sweep rule moved into `_pin_complaints` so a fixture runs through the same code the inventory does, and the perturbations are committed: a contradictory duplicate inside one sweep **in both row orders** (only one of them ever failed), an uncorroborated pin, a resolved subject left `unpinned`, an `open` PR reserved with a real origin, and a misspelled reserve origin. Each was confirmed to fail against the reverted implementation and pass against the fixed one. **[P3] Three consistency fixes.** The `#3076` row's note still instructed the reader to resolve both SHAs and led with the discarded `5ea9a0e8` PR-head abbreviation; it now states the resolution. The close-out's run-table row still counted three candidates as three servers. And the inventory carried two current statuses: the opening paragraph, the origin-floor reading and the order-of-work line all still described sourcing as future work. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Summary
Sourcing for the
pre_1_0corpus is finished. The strata inventory goes from 59 of 60 slots with a candidate (three of them unpinned) to 60 of 60pinned, so nothing in it needs mining before the Cut C calibration round. One sweep —2026-W36-closeout, 45 rows over three repositories — does all of it.langchain_crewai × insufficient_evidenceis claimed bybytedance/deer-flow#4868. Session B's lead was to stop mininglangchain-mcp-adaptersand find an application that callsMultiServerMCPClient/load_mcp_toolsat agent construction; deer-flow is one, and the claimed PR adds per-user credential injection for shared MCP servers, so which credential a tool call carries is chosen at run time from the caller's identity and a$ENV_VARmap. Neither the servers, the tools, nor the credentials are in the tree.github-mcp-server#3020,#3076andgrafana/mcp-grafana#1080were carried from adoption walks with abbreviated or absent SHAs, and the inventory said they had to be pinned before Cut C. Each was mined by number, which resolves both ends under the sweep convention and puts the pins where a guard can re-read them.test_a_pinned_external_candidate_matches_the_sweep_that_recorded_itwas added in Cut A and deleted by Cut A's own review commit (958a9073) — the same commit that added the prose citing it by name. Restored and generalized: every external pin is re-read from the sweep that resolved it, a pin no sweep corroborates is refused, a subject a sweep did resolve may not situnpinned, and two recordings of one subject that disagree fail whether they sit in two sweeps or twice in one file.Things a reviewer should know
#3076is why re-reading pins matters. Its walk note recorded head5ea9a0e8…; that isrefs/pull/3076/head, and after a squash merge it is not an ancestor of the default branch at all. Resolving the abbreviation "correctly" would have pinned an object no clone reaches. The merge commit is8ec62491…, and its first parent is thebfb59bb7…the same note recorded — half the pin was right, which is the dangerous half.init_skipbecause "the example trees are no longer picked up by cold-startinit". Reproduced at the recorded pin, the actual result isrefused_unresolved_scope: the repository holds more than one self-contained project that defines agents, and one manifest describes one agent surface. The refusal is the monorepo behavior working. What the miner cannot do is act on it —_run_initreads only "did a manifest appear", so a refusal that names the right next step is indistinguishable in the CSV from a crash.init_skipfor that reason; pointed atbackend/packages/harness,init --writewrites a manifest and the scan reaches a decision. Nine slots claimed before this cut are in the same position. A receipt is rooted at a project, not at a clone, and the inventory now says so.--limit 40is not a time window. deer-flow's latest forty merged PRs span six days; crewAI-examples' forty reach back eighteen months. The claimed candidate is five days older than the window's oldest PR, which is why it took--prrather than a wider sweep — and why a busy repository's silence under--limit 40means very little.diff_substance. The walks are what targeted their cells; being swept afterwards addsbenchmark_scoredto their exposure and changes nothing about what aimed them. All three were alreadytuning_onlythroughmaintainer_walk, so no cell's holdout margin moved.test_a_gap_that_names_a_pull_request_plans_the_origin_that_pr_can_supplyends withassert checked, "no gap names a pull request any more; drop this guard or restore one". It is re-pointed at the Reserve table, which states an origin beside a state exactly the way a gap did and is the pool a relabel draws on — 33 claims checked, and a reserve origin a state cannot supply now fails.#4868is one session's cell-targeting label, from the diff, and not adjudicated — like every other sourcing label underbenchmark/miner/results/. Corpus labels come only from the Amendment 1 raters. The register records the counter-reading with it: the feature's default is fail-closed, so a rater may weigh the tightening rather than the unenumerable surface.Review of this branch, and what it changed
Round 2 (
8ff68ef5) — five findings, all addressed:*-minedruns were integrity-checked._swept_pinsreads every committed CSV, whiletest_miner_corpus.pyvalidated well-formedness, CSV/JSONL parity and LF endings over*-mined.jsonlalone — so a half-regenerated-cutb,-closeout,-reevalorconstructedpair could leave stale pins certifying the inventory. The three integrity guards now run over every committed run; only the KPI aggregation stays on*-mined. Two guards keep the split honest: the enumerations may not converge, and a results CSV with no JSONL sibling fails._pin_complaints, so fixtures run through the same code the inventory does. The perturbations are committed — a contradictory duplicate inside one sweep in both row orders, an uncorroborated pin, a resolved subject leftunpinned, anopenPR reserved with a real origin, a misspelled reserve origin — and each was confirmed by reverting the implementation, not by construction.#3076row's note still carried a completed instruction; the run-table row still counted three candidates as three servers; and the inventory carried two current statuses (the opening paragraph, the origin-floor reading and the order-of-work line all still described sourcing as future work).Round 1 (
89cc708b) — two of the guards this PR adds failed open on the shape they exist for:_swept_pinskeyed recordings by sweep file, so one file holding a subject twice with different SHAs kept only the last row — a contradictory duplicate ordered before the good row read as agreement. Confirmed by replaying the pre-fix keying against that perturbation. Now keyed by the pins, with the sweeps that recorded them as the value._reserve_claimsskipped every reserve row whoseStatewas outsideSTATE_ORIGINS, which includesopen— soadk-samples#1745, the candidate that taught this project that an open PR is not history, could have been reserved asreal_historyand passed. A row that states an origin must now state a state that can supply one.Two prose claims were also overstated and corrected (
86227ccb): "the three walked MCP servers" counted three candidates as three servers, and the deer-flow window figure said six weeks where the truth is five days.Type
src/changesVerification
CI is authoritative for
python -m ruff check .,python -m compileall -q src tests, andpython -m pytest. All checks pass on the head commit.Additional local checks run:
pytest -n auto -m "not perf"on the full suite: exit 0;ruff check .: clean;compileall -q src tests: clean.continue-on-unknown-state fails both reserve cases. The whole-file perturbation sweep still runs clean against the committed tree (eight perturbations, all fail closed), and a one-character edit to the closeout CSV now failstest_csv_and_jsonl_agree[2026-W36-closeout.jsonl].#3020115 → 116 checked-in tool snapshots,#3076116 → 117,mcp-grafana#1080one addedMustToolregistration;#3076's5ea9a0e8…confirmed asrefs/pull/3076/headand unreachable frommain.init/scanreproduced by hand on the deer-flow head atbackend/packages/harness(manifest written, scan reaches a decision) and at the repository root (refused_unresolved_scope), and on openai-agents-python at its own recorded pin (same refusal).Release-readiness notes
benchmark/, already network-touching by design)docs/checks.md— none changedSTABILITY.md— noneCloses nothing on its own: #456 stays open for Cut C (calibration, labels, freeze), Cut D (receipts) and Gate 2.
🤖 Generated with Claude Code