Skip to content

[issue-2817][slice-1/28] Docs and seam-table scaffolding - #2902

Closed
james-in-a-box[bot] wants to merge 6 commits into
egg/issue-2817/workfrom
egg/issue-2817/slice-1
Closed

[issue-2817][slice-1/28] Docs and seam-table scaffolding#2902
james-in-a-box[bot] wants to merge 6 commits into
egg/issue-2817/workfrom
egg/issue-2817/slice-1

Conversation

@james-in-a-box

Copy link
Copy Markdown
Contributor

Closes #2817 (refresh of #2261). The make lint step caps Python
source files at 1500 lines / 100 KB; 17 files are still
grandfathered in scripts/file-size-allowlist.yaml. Six prior
pipeline attempts on #2261 failed to land any follow-up slice after
the pattern + worked reference merged via #2335. This plan
decomposes all 17 files following the canonical pattern in
docs/guides/decomposition-pattern.md (sub-package + explicit
per-symbol re-export barrel + underscore-prefixed private
submodules; scripts/select_tests/ is the worked reference).

The work breaks into 28 slices in a single forest-shaped DAG:

  1. slice-1 (root, docs/scaffolding) retags stale #2261
    references to #2817 across the pattern doc, both CLAUDE.md
    seam tables, and the allowlist comment blocks (cq-5, cq-6);
    updates the pattern doc for the cq-4 relaxation of
    non-negotiable ci: bump actions/checkout from 4 to 6 #10; adds a Submodule seam table to
    sandbox/CLAUDE.md and creates shared/CLAUDE.md with one
    (cq-2); seeds TBD rows for kubernetes_spawner.py and
    routes/phases.py. No source decomposition lands here.
  2. slices 2–7 decompose orchestrator/routes/pipelines.py as a
    linear chain. Slice-2 lands the step-0 baseline + five flat
    clusters. Slices 3-6 each extract one sub-sub-package
    (_prompt_building/, _pr_lifecycle/, _worktree_ops/,
    _concurrent_phase/). Slice-7 is terminal: carves
    _run_pipeline into per-phase handlers via a two-stage
    refactor (PHASE_HANDLERS dispatch + per-phase module lift),
    drops the allowlist entry, and fills the seam-table row.
  3. slices 8–12 decompose gateway/gateway.py as a linear
    chain (baseline + _app_factory / _auth / _checkpoint_routes
    / _gh_routes; then _git_routes/; then _jira_routes/;
    then _confluence_routes//_worktree_routes/_anthropic_proxy;
    terminal _sessions + allowlist drop). Non-negotiable Merge Phase 1.5 docs into main #8
    keeps every @app.route decorator in __init__.py.
  4. slices 13–28 are independent per-file decompositions that
    all root at slice-1. Each follows the canonical 4-task shape
    (step-0 git mv baseline → external-importer audit → cluster
    extraction → allowlist drop + seam-table fill). Slices 22-23
    are a chained pair: slice-22 breaks the 885-LOC
    write_health_summary method in-place into named helpers
    (PREP); slice-23 mechanically extracts submodules (TERMINAL).

Impact: every file under the global cap; the allowlist's files:
map empties; the seam tables in orchestrator/CLAUDE.md,
gateway/CLAUDE.md, the new sandbox/CLAUDE.md, and the net-new
shared/CLAUDE.md carry concrete submodule layouts. Pure refactor
— no behaviour change. Test patch targets (49 distinct symbols
across routes.pipelines.*, 6 across gateway.gateway.*, 0
across mcp_tools.*) continue resolving through the barrel
__init__.py. Downstream consumer #2562 (restore coder/reviewer
full check-suite access) becomes unblocked; #2319 (per-phase
handler unit tests) can start once slice-7 lands.

This slice

Docs and seam-table scaffolding

Files affected:

  • docs/guides/decomposition-pattern.md
  • orchestrator/CLAUDE.md
  • gateway/CLAUDE.md
  • sandbox/CLAUDE.md
  • shared/CLAUDE.md
  • scripts/file-size-allowlist.yaml

Tasks:

  • task-1-1: Update docs/guides/decomposition-pattern.md to reflect the cq-4 relaxation of non-negotiable ci: bump actions/checkout from 4 to 6 #10. Add a short subsection explaining that scenario-organized test suites (e.g. select_tests/) may stay topical rather than being force-fit to a per-submodule layout; the load-bearing requirement is that test files use the barrel surface (patch("foo._bar")) so they don't move when source internal layout shifts. Add a note in the Pre-merge checklist that 1:1 test mirroring is guidance, not a gate. Leave the canonical recipe sections (a)-(h) unchanged in shape.
    • Acceptance criteria: docs/guides/decomposition-pattern.md gains a new subsection (or paragraph) explaining the cq-4 relaxation in the non-negotiable ci: bump actions/checkout from 4 to 6 #10 area; the Pre-merge checklist references the relaxation; reviewer_documenter ACK.
  • task-1-2: Retag all stale Decompose 15 oversize Python source files to clear the file-size allowlist #2261 references to Decompose 17 oversize Python source files to clear the file-size allowlist #2817 across docs/guides/decomposition-pattern.md (5 hits at lines 5, 26, 261, 276, 308 — preserve the docstring's historical "refresh of Decompose 15 oversize Python source files to clear the file-size allowlist #2261" pointer if any), orchestrator/CLAUDE.md (11 hits at lines 18, 20, 22, 40, 46, 50-55), and gateway/CLAUDE.md (8 hits at lines 17, 19, 21, 33, 39, 43-45). Each occurrence becomes Decompose 17 oversize Python source files to clear the file-size allowlist #2817 in the same syntactic context (issue link, slice label, parent reference). Keep the slice numbers from the prior Decompose 15 oversize Python source files to clear the file-size allowlist #2261 plan (e.g. "slice-12") only if the planner has chosen to preserve them — otherwise mark them TBD in line with this plan's slice IDs.
  • task-1-3: Add a "Submodule seam tables" section to sandbox/CLAUDE.md (currently 11 lines, no seam table). Section mirrors the existing convention in orchestrator/CLAUDE.md and gateway/CLAUDE.md: header, a paragraph pointing at docs/guides/decomposition-pattern.md, then a primary table for in-flight decompositions with TBD rows for sandbox/egg_lib/orch_cli.py (slice-13) and sandbox/entrypoint.py (slice-19).
    • Acceptance criteria: sandbox/CLAUDE.md has a "Submodule seam tables" section with header text matching the orchestrator/CLAUDE.md convention and two TBD rows for the two in-scope sandbox files. reviewer_documenter ACK.
  • task-1-4: Create the net-new file shared/CLAUDE.md with a header (subsystem name + 1-line summary), a pointer to docs/index.md and docs/guides/decomposition-pattern.md, and a "Submodule seam tables" section with TBD rows for shared/egg_contracts/checkpoint_cli.py (slice-17) and shared/egg_contracts/plan_parser.py (slice-25).
    • Acceptance criteria: shared/CLAUDE.md exists with the required sections; TBD rows present for both in-scope shared/ files. reviewer_documenter ACK.
  • task-1-5: Add TBD rows to orchestrator/CLAUDE.md's in-flight decomposition table for orchestrator/kubernetes_spawner.py (slice-24) and orchestrator/routes/phases.py (slice-27). Refresh the line-count column in the table with current wc -l numbers for the existing rows (mcp_tools.py 2820, gateway_client.py 3713, overseer/monitor.py 2024, peer_consensus.py 2215, routes/signals.py 2557, routes/deployment.py 1650).
    • Acceptance criteria: orchestrator/CLAUDE.md in-flight table has rows for all 8 orchestrator-side files in scope (the 6 existing rows refreshed + 2 new). reviewer_documenter ACK.
  • task-1-6: Retag allowlist comment blocks in scripts/file-size-allowlist.yaml (cq-5): (a) orchestrator/routes/phases.py entry: issue: "2261" becomes issue: "2817"; the 7-line comment block above the entry has its #2261 references rewritten to #2817 (and the "slice-15 cluster" phrase updated to reflect this plan's slice numbering — slice-27). (b) shared/egg_contracts/plan_parser.py entry: issue: "2548" becomes issue: "2817"; the 7-line comment block has its #2548/#2569 references rewritten to #2817 (the entry is removed entirely when slice-25 decomposes plan_parser.py; this retag is interim). Do NOT remove either allowlist entry in this slice — only the comment text and issue: fields change.
    • Acceptance criteria: git diff scripts/file-size-allowlist.yaml shows ONLY comment + issue: field changes (no entry additions or removals). make lint is green. Both entries still present in files: map.

Test Plan

Automated:

  • make lint on every slice PR: ratchet check via
    scripts/check-file-sizes.py confirms strict size decrease for
    the touched file; pre-commit equivalents (ruff, ruff-format)
    run via Makefile:201lint-custom:258-273 glob loop.
  • make test-all on every slice PR: full pytest suite, mypy,
    and custom checks; reviewer confirms the existing 49 +
    6 distinct test-patch targets continue resolving through the
    barrel __init__.py (no test file moves required because
    non-negotiable ci: bump actions/checkout from 4 to 6 #10 is relaxed per cq-4).
  • Pre-extraction audit per slice: git grep -nE "(<module>\.|from .* import .*\b<symbol>\b)" -- '*.py'
    (unanchored — catches in-function lazy imports). Audit
    deliverable in the slice PR description.
    Manual:
  • Slice-7 (terminal pipelines.py): human-operator runs a
    small refine→plan→implement→pr SDLC pipeline against a
    doc-only issue and confirms no behaviour change in BRC
    consensus, signal handling, slice-DAG admission, HITL gates.
    This is the RISK-2 safety net.
  • Slice-12 (terminal gateway.py): security_review ACK
    required (gateway sits on the policy-enforcement seam);
    operator confirms the 47 @app.route(...) decorators are
    still in __init__.py and that no decorator moved into a
    submodule (non-negotiable Merge Phase 1.5 docs into main #8).
  • Slices 9 and 21 (security-adjacent: _git_routes/ carve-out
    from gateway.py / git_client.py): security_review ACK
    required.

Manual Steps

Pre-merge: none beyond standard reviewer ACK + HITL approvals per
BRC consensus protocol.

Post-merge (after slice-28 lands and scripts/file-size-allowlist.yaml
files: map is empty):

  1. Operator closes Decompose 17 oversize Python source files to clear the file-size allowlist #2817 referencing the 28 merged slice PRs.
  2. Operator notifies Restore full-check-suite guidance (pipeline coder/reviewer + CI PR-bot agents) once #3312 lands #2562 (restore coder/reviewer full check-suite
    access) — that issue becomes unblocked.
  3. Operator notifies Add unit tests for per-phase handlers extracted from _run_pipeline (#2261 follow-up) #2319 (per-phase handler unit tests) — those
    tests can be authored against the now-extracted
    _run_refine.py / _run_plan.py / _run_implement.py /
    _run_pr.py modules.

Stack

  • Position: slice 1 of 28 in pipeline issue-2817
  • Stacked on top of egg/issue-2817/work

Slice slice-1 of pipeline issue-2817. Stacked on top of egg/issue-2817/work.

egg and others added 4 commits May 31, 2026 03:55
Retag the two oversize-Python allowlist comment blocks that pre-dated
the #2817 program:

* orchestrator/routes/phases.py: comment block + `issue:` field move
  from #2261 to #2817; "slice-15 cluster" / "slice-15 targets" become
  "slice-27" to match this plan's slice numbering for phases.py
  decomposition (slice-27 in the #2817 DAG).
* shared/egg_contracts/plan_parser.py: comment block + `issue:` field
  move from #2548 (allowlist parent) / #2569 (decomposition follow-up)
  to #2817; the historical `egg/issue-2548/work` branch-name reference
  is preserved verbatim since it is a literal git ref and not a
  #-tagged issue reference. The `#2527` reference is also preserved
  (out of task scope).

Both entries remain in `files:` map. They will be removed entirely by
slice-25 (plan_parser.py decomposition) and slice-27 (phases.py
decomposition). No source decomposition lands in this slice (slice-1
is docs / seam-table scaffolding).

`python3 scripts/check-file-sizes.py` exits 0 (allowlist parses
cleanly; only pre-existing soft-cap warnings are emitted, unrelated
to this change). `make lint` venv-sync was blocked in the sandbox by
a transient PyPI TLS failure on `mypy==1.19.1`; CI will run the full
lint pipeline against this commit.

Task: task-1-6
Slice: slice-1
…ld sandbox/shared seam tables

Slice-1 is the docs/scaffolding root of the 28-slice DAG for #2817; no
source decomposition lands. This commit covers the 5 documenter tasks:

- TASK-1-1: docs/guides/decomposition-pattern.md gains section (i)
  documenting the cq-4 relaxation of non-negotiable #10 — test layout
  is guidance, not a 1:1 gate; scenario-organized suites may stay
  topical so long as test files use the barrel surface. Pre-merge
  checklist updated to reference the new section.
- TASK-1-2: all stale #2261 references in the pattern doc,
  orchestrator/CLAUDE.md, and gateway/CLAUDE.md are retagged to #2817.
  The historical "refresh of #2261" pointer in the pattern doc's
  docstring is preserved as the single allowed exception.
- TASK-1-3: sandbox/CLAUDE.md gains a "Submodule seam tables" section
  with TBD rows for sandbox/egg_lib/orch_cli.py (slice-13) and
  sandbox/entrypoint.py (slice-19).
- TASK-1-4: net-new shared/CLAUDE.md is created with subsystem header,
  doc pointers, and a "Submodule seam tables" section carrying TBD
  rows for shared/egg_contracts/checkpoint_cli.py (slice-17) and
  shared/egg_contracts/plan_parser.py (slice-25).
- TASK-1-5: orchestrator/CLAUDE.md's in-flight table is refreshed with
  current wc -l sizes and slice IDs from this plan; new TBD rows are
  added for orchestrator/kubernetes_spawner.py (slice-24) and
  orchestrator/routes/phases.py (slice-27).

Gateway/CLAUDE.md's in-flight table is similarly refreshed with current
sizes and slice IDs (slice-20/21/26), and the gateway/gateway/ TBD
table is extended with the additional pre-allocated clusters from the
plan (_confluence_routes, _worktree_routes, _anthropic_proxy,
_checkpoint_routes, _gh_routes).

No source decomposition is included; cq-5 retag of the allowlist YAML
comment block is TASK-1-6 (coder role).
Address reviewer_code's non-blocking nit on the gateway/CLAUDE.md
prose: slice-11 extracts three sub-sub-packages (_confluence_routes/,
_worktree_routes, _anthropic_proxy), not one. The TBD table already
listed all three correctly; this aligns the narrative paragraph with
the architect-slices.yaml plan.

@egg-reviewer egg-reviewer Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No agent-mode design concerns. This slice is pure docs + seam-table scaffolding (retags #2261#2817, adds section (i) on test-layout guidance, seeds sandbox/CLAUDE.md and shared/CLAUDE.md seam tables, updates allowlist comments). No agent prompts, model calls, output parsing, or sandbox-boundary touches are in scope.

— Authored by egg

@james-in-a-box

This comment has been minimized.

@egg-reviewer egg-reviewer Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed all six modified human-authored files (the two .egg-state/brc-history/* artifacts are skipped per review conventions). The PR matches its stated scope, the lint passes (python3 scripts/check-file-sizes.py exits 0), and every claim is verifiable:

  • All file line counts in orchestrator/CLAUDE.md and gateway/CLAUDE.md agree with wc -l (within the documented ~ rounding).
  • Both allowlist entries (shared/egg_contracts/plan_parser.py, orchestrator/routes/phases.py) remain present; only comment text and issue: fields changed.
  • The example imports in the new/expanded CLAUDE.md files resolve to real symbols (from egg_lib.orch_cli import mainsandbox/egg_lib/orch_cli.py:4011; from egg_contracts.checkpoint_cli import mainshared/egg_contracts/checkpoint_cli.py:2215; from egg_contracts.plan_parser import parse_planshared/egg_contracts/plan_parser.py:1267).
  • Slice-ID assignments (slice-13…slice-28 per-file decompositions) match the PR-body DAG.
  • Only two #2261 references remain in the three retagged docs, both inside the explicitly-preserved "refresh of #2261" pointer on lines 5–6 of docs/guides/decomposition-pattern.md (per task-1-2).

No blocking issues. Posting as --comment because this is a self-authored bot PR.

Non-blocking findings below.

Non-blocking

1. gateway/CLAUDE.md: cluster naming inconsistency

The seam table at lines 31–42 and the narrative paragraph at lines 19–29 are inconsistent on the / (sub-sub-package) vs .py (flat module) vs bare-name (ambiguous) suffix convention:

  • Explicit sub-sub-packages: _git_routes/, _jira_routes/, _confluence_routes/
  • Explicit flat modules: _auth.py, _sessions.py, _app_factory.py
  • Ambiguous (no suffix): _worktree_routes, _anthropic_proxy, _checkpoint_routes, _gh_routes

The patch 3/4 commit message claims slice-11 extracts three sub-sub-packages (_confluence_routes/, _worktree_routes, _anthropic_proxy), but the doc only puts a trailing / on the first. Either the table-and-narrative are wrong (the latter two should have trailing slashes) or the commit message is wrong (they're flat). Pick one convention and apply it to all rows so contributors landing slice-11 know the target shape.

2. scripts/file-size-allowlist.yaml: comment-block factual drift from mechanical retag

The retag rewrote historical (#2548) and (#2569) parenthetical references inside the explanatory comments, not just the load-bearing issue: field. After this PR, lines 53–59 read:

# On the egg/issue-2548/work merge target, slice-1's
# extract_pr_context_metadata_from_yaml + ParseResult.pr_context_*
# plumbing (#2817) stacks on top of #2527's validate_task_role_alignment
# additions, ...
# ... Allowlisting under #2817 so the
# BRC implement-phase lint passes; decompose under #2817.

That plumbing was actually added under #2548 (the preserved egg/issue-2548/work branch reference on the first line still attests to that); the (#2817) claim on line 55 is now historically wrong and contradicts the preserved branch reference. Same pattern on lines 62–68: slice-15 cluster in #2261 was rewritten to slice-27 cluster in #2817, but the cluster was historically tracked under #2261 — the retag erases that provenance.

The cleaner pattern would have been to only retag the load-bearing fields (the issue: keys, which are what the program tracker reads) and leave the parenthetical history alone. The drift here has a short half-life because slices 25/27 will delete these entries entirely, but worth noting for future similar retags.

3. Stale test-fragility notes still point at #2261 slice-15

orchestrator/tests/test_populate_contract_audit_events.py:1217 and :1341 carry "Fragility note (#2261 slice-15)" pointing at the _run_pipeline per-phase-handler decomposition. Under #2817 that work is slice-7 (per the seam table at orchestrator/CLAUDE.md:31 and the PR body). Out of task-1-2's scope as written, but once #2261 closes these notes will dead-link a reader. Fold into slice-7's PR (which will be touching these lines anyway) or file a follow-up.

4. Stale "15 slices" reference in pattern doc's See-also

docs/guides/decomposition-pattern.md:354–355 still describes the slice-DAG as driving "the 15 slices in parallel." This plan is 28 slices in a forest-shaped DAG (per the PR body's first paragraph). Pre-existing prose, not introduced by this PR, but the surrounding paragraph just got retagged from #2261#2817 so the count is the only remaining anchor to the old plan. Worth a one-line fix while the area is hot.

— Authored by egg

@james-in-a-box

This comment has been minimized.

@james-in-a-box

This comment has been minimized.

- gateway/CLAUDE.md: make sub-sub-package (/) vs flat module (.py) suffix
  convention consistent across the narrative paragraph and the seam table.
  Slice-11 extracts three sub-sub-packages per the commit message —
  _confluence_routes/, _worktree_routes/, _anthropic_proxy/ — and
  _checkpoint_routes.py / _gh_routes.py are flat modules.
- scripts/file-size-allowlist.yaml: restore historical parenthetical
  references in comment blocks. Only the load-bearing issue: fields stay
  rewritten to 2817; the comments preserve their original #2548/#2569 and
  slice-15/#2261 history so they no longer contradict the preserved
  egg/issue-2548/work branch reference. Each block adds a short pointer
  to the current issue: field so the cross-reference stays discoverable.
- orchestrator/tests/test_populate_contract_audit_events.py: retag the
  two Fragility notes from (#2261 slice-15) to (#2817 slice-7) — the
  per-phase handler decomposition is now slice-7 of #2817 per the
  orchestrator/CLAUDE.md seam table.
- docs/guides/decomposition-pattern.md: See-also bullet now says 28
  slices (matching the actual plan) instead of the stale 15-slice count
  inherited from the #2261-era plan.
@james-in-a-box

Copy link
Copy Markdown
Contributor Author

Thanks for the careful review. All four non-blocking findings addressed in 6615c10.

Per-item disposition

1. gateway/CLAUDE.md: cluster naming inconsistencyfixed-in-PR (commit 6615c10)
Aligned the narrative paragraph and the seam table on a single convention: trailing / denotes a sub-sub-package, .py denotes a flat module, no bare names. Per the slice-11 commit message, _confluence_routes/, _worktree_routes/, and _anthropic_proxy/ are all sub-sub-packages; _checkpoint_routes.py and _gh_routes.py are flat modules. Added a one-line legend after "Pre-allocated submodule clusters per the plan" so contributors landing slice-11 see the convention inline.

2. scripts/file-size-allowlist.yaml: comment-block factual drift from mechanical retagfixed-in-PR (commit 6615c10)
Restored the historical parentheticals in both comment blocks. Only the load-bearing issue: fields stay rewritten to 2817; the comments preserve their original (#2548) / (#2569) and slice-15 / #2261 history so they no longer contradict the preserved egg/issue-2548/work branch reference. Each block now adds a short pointer to the current issue: field (now tracked under #2817 — see issue: field) so the cross-reference stays discoverable while the historical truth is intact.

3. Stale test-fragility notes still point at #2261 slice-15fixed-in-PR (commit 6615c10)
Updated both Fragility notes in orchestrator/tests/test_populate_contract_audit_events.py (lines 1217 and 1341) from (#2261 slice-15) to (#2817 slice-7), matching the orchestrator/CLAUDE.md seam-table slice ID for the per-phase handler decomposition. Folded into this PR rather than waiting for slice-7 since the reviewer's "fold or file" framing left it open, and an in-PR fix avoids another follow-up bouncing through the queue.

4. Stale "15 slices" reference in pattern doc's See-alsofixed-in-PR (commit 6615c10)
One-line fix in docs/guides/decomposition-pattern.md:35415 slices28 slices to match the actual plan's forest-shaped DAG count called out in the PR body's first paragraph.

Verification

  • python3 scripts/check-file-sizes.py exits 0
  • YAML parses cleanly (python3 -c "import yaml; yaml.safe_load(open('scripts/file-size-allowlist.yaml'))")
  • make lint ruff/format clean; the 2 mypy errors in shared/egg_agent/client.py:387-388 are pre-existing on this branch (file untouched by this PR)

— Authored by egg

@james-in-a-box

This comment has been minimized.

@egg-reviewer egg-reviewer Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No agent-mode design concerns.

Delta since c789c2c is doc/comment hygiene only — slice count fix in decomposition-pattern.md, sub-sub-package vs flat-module suffix convention in gateway/CLAUDE.md's seam table, #2261 slice-15 → #2817 slice-7 fragility-note retag in test_populate_contract_audit_events.py, and restoration of historical parenthetical references in file-size-allowlist.yaml comment blocks. None of these change how agents are prompted, what context they receive, how their output is consumed, or any sandbox/API boundary.

— Authored by egg

@james-in-a-box

This comment has been minimized.

@egg-reviewer egg-reviewer Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed the delta (c789c2c..6615c105, 4 files changed). Three of my four previous non-blocking findings are addressed cleanly; one is addressed but in the wrong direction and now contradicts the canonical architect plan.

Per-finding disposition

# Finding Status
1 gateway/CLAUDE.md cluster suffix convention Not resolved — fix introduces a new factual error (see below)
2 Allowlist YAML comment drift Resolved. Historical (#2548) / (#2569) / slice-15 cluster in #2261 parentheticals are restored; load-bearing issue: fields stay rewritten to "2817"; the "see issue: field" cross-references make the current tracker discoverable.
3 Stale test fragility notes Resolved. Both (#2261 slice-15)(#2817 slice-7) retags on orchestrator/tests/test_populate_contract_audit_events.py:1217 and :1341 match the seam-table slice ID.
4 "15 slices" in pattern doc See-also Resolved. docs/guides/decomposition-pattern.md:354 now says 28 slices.

Blocking — gateway/CLAUDE.md finding #1 re-introduced in a worse form

The fix aligned the narrative paragraph and the seam table on a new convention (/ denotes sub-sub-package, .py denotes flat module — explicit legend on lines 29–30). The convention itself is fine. But the classification of _worktree_routes and _anthropic_proxy is wrong:

gateway/CLAUDE.md:26-27 (narrative) now reads:

"slice-11 extracts three (_confluence_routes/, _worktree_routes/, and _anthropic_proxy/)"

gateway/CLAUDE.md:37-38 (table rows) now list:

_worktree_routes/ … worktree lifecycle endpoints
_anthropic_proxy/ … Anthropic API proxy + UpstreamRegistry routing

The canonical architect plan (.egg-state/agent-outputs/2817-architect-slices.yaml:215-237, which the PR description names as the program's source-of-truth) is unambiguous that only one of the three is a sub-sub-package:

  - id: 11
    name: |-
      gateway.py _confluence_routes/ + _worktree_routes + _anthropic_proxy
    goal: |-
      ... into gateway/gateway/_confluence_routes/. Also extract
      _worktree_routes.py (get_worktree_manager + map_container_path_to_worktree
      + _cleanup_stale_pack_files + _cleanup_empty_container_dir +
      worktree_create / delete / list + ...) and _anthropic_proxy.py (~1K LOC:
      get_anthropic_client + _get_forwarded_headers + ...).

The plan draft (.egg-state/drafts/2817-plan.md:1167-1187) repeats the .py extension verbatim for both:

"Extract gateway/gateway/_worktree_routes.py. Thin..."
"Extract gateway/gateway/_anthropic_proxy.py (~1K LOC, ...)"

So slice-11 actually extracts one sub-sub-package + two flat modules, not three sub-sub-packages. The pre-fix state was ambiguous (no suffix); the fix has now made gateway/CLAUDE.md confidently wrong against the canonical plan.

The root cause is that the fix commit message cites the earlier CLAUDE.md prose commit (3662b07) as authority — but that commit itself was internally inconsistent ("three sub-sub-packages (_confluence_routes/, _worktree_routes, _anthropic_proxy)" — only the first had a trailing slash). My previous review compounded the confusion by framing the fix as a convention-alignment problem rather than a fact-check against the architect plan. Apologies — that's on me. The architect plan was the right source to consult, and on that read, the answer is clear.

Suggested fix

Three coordinated edits in gateway/CLAUDE.md:

  1. Narrative (line 25-27) — change "slice-11 extracts three (\_confluence_routes/`, `_worktree_routes/`, and `_anthropic_proxy/`)"` to:
slice-11 extracts one sub-sub-package (`_confluence_routes/`) plus two
flat modules (`_worktree_routes.py`, `_anthropic_proxy.py`)
  1. Table row (line 37)_worktree_routes/_worktree_routes.py

  2. Table row (line 38)_anthropic_proxy/_anthropic_proxy.py

The legend on lines 29–30 stays. With these three edits, the prose + table + plan all agree.

Why this matters even though slice-11 is several steps away: gateway/CLAUDE.md is the contributor-facing seam table — it's what a future agent picks up when claiming slice-11. If the documented shape disagrees with the architect plan, the slice-11 PR will either (a) build sub-sub-packages and get NACK'd against the plan, or (b) catch the discrepancy and need a follow-up to fix this doc. The PR is already touching these exact lines — fix it in-PR rather than queuing another bounce. The rows are marked TBD elsewhere, but the narrative paragraph is a definitive claim ("slice-11 extracts three [sub-sub-packages]") that is factually false.

Verification of resolved items

  • YAML allowlist (scripts/file-size-allowlist.yaml:53-72): Diff vs base branch shows issue: field correctly bumped from "2548""2817" and "2261""2817"; comment parentheticals preserve historical (#2548), (#2569), (slice-15 cluster in #2261). The "now refreshed under #2817 — see issue: field" cross-references resolve the apparent contradiction with the preserved egg/issue-2548/work branch reference. Note: the line-67 phrase "follows the same pattern as the other slice-15 targets" is a vestigial reference (no other slice-15 targets remain in the file), but this was already present on the base branch — not a PR regression.
  • Test fragility notes: Both class-level fragility notes (TestPlanCompleteCallSiteWireUp:1217, TestSafetyNetForestViolationLandsOnEmptyContractHitl:1341) correctly point at (#2817 slice-7), matching orchestrator/CLAUDE.md seam table row for _run_loop/.
  • docs/guides/decomposition-pattern.md:354: 15 slices28 slices matches the PR body's first paragraph.

Posting as --comment since this is a self-authored bot PR; treat the gateway/CLAUDE.md finding as a should-fix-before-merge.

— Authored by egg

@james-in-a-box

This comment has been minimized.

@james-in-a-box

This comment has been minimized.

The previous fix aligned narrative + table on a single convention, but
mis-classified _worktree_routes and _anthropic_proxy as sub-sub-packages.
Per .egg-state/agent-outputs/2817-architect-slices.yaml:215-237 and
.egg-state/drafts/2817-plan.md:1167,1178, slice-11 actually extracts
one sub-sub-package (_confluence_routes/) plus two flat modules
(_worktree_routes.py, _anthropic_proxy.py). Three coordinated edits in
gateway/CLAUDE.md bring the prose, table, and architect plan into agreement.
@james-in-a-box

Copy link
Copy Markdown
Contributor Author

Thanks for the careful re-review and the apology — the architect plan was indeed the right authority to consult, and on that read the answer is unambiguous: slice-11 extracts one sub-sub-package plus two flat modules.

Per-item disposition

1. gateway/CLAUDE.md slice-11 misclassification (re-opened)fixed-in-PR (commit 81c5b8a)
Applied the three coordinated edits exactly as suggested. Confirmed against both sources of truth:

  • .egg-state/agent-outputs/2817-architect-slices.yaml:215-237 — goal text says _worktree_routes.py and _anthropic_proxy.py (~1K LOC: ...), with only _confluence_routes/ going into a sub-sub-package directory.
  • .egg-state/drafts/2817-plan.md:1167,1178 — repeats the .py extension verbatim for both flat modules.

Specifically:

  • Narrative (lines 25–28): now reads "slice-11 extracts one sub-sub-package (_confluence_routes/) plus two flat modules (_worktree_routes.py, _anthropic_proxy.py)".
  • Table row (line 38): _worktree_routes/_worktree_routes.py.
  • Table row (line 39): _anthropic_proxy/_anthropic_proxy.py.

The legend on lines 29–30 (trailing / = sub-sub-package, .py = flat module) stays — prose + table + architect plan are now all in agreement. A future slice-11 picker reads the contributor-facing seam table and sees the correct target shape inline.

Verification

  • python3 scripts/check-file-sizes.py exits 0
  • git diff --stat confirms scope: only gateway/CLAUDE.md touched (5 insertions, 4 deletions) — no other files affected.

— Authored by egg

@james-in-a-box

This comment has been minimized.

@egg-reviewer egg-reviewer Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No agent-mode design concerns.

The delta since my last review (commit 81c5b8a) is a 5-line documentation fix in gateway/CLAUDE.md correcting the slice-11 module classification (_worktree_routes and _anthropic_proxy are flat .py modules, not sub-sub-packages) to match the architect plan. This is a docs-only consistency fix in a TBD seam table — no agent-facing surface, prompts, or pipeline behavior changes.

— Authored by egg

@egg-reviewer egg-reviewer Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed the delta (6615c105..81c5b8a6, 1 file changed) against the prior blocking finding.

Per-finding disposition

# Prior finding Status
1 gateway/CLAUDE.md slice-11 misclassification (_worktree_routes / _anthropic_proxy shown as sub-sub-packages contrary to architect plan) Resolved.

Verification

The three coordinated edits suggested in my previous review have been applied exactly:

  • Narrative (lines 25–28) now reads "slice-11 extracts one sub-sub-package (_confluence_routes/) plus two flat modules (_worktree_routes.py, _anthropic_proxy.py)" — matches .egg-state/agent-outputs/2817-architect-slices.yaml:226-237, which spells out _worktree_routes.py (get_worktree_manager + ...) and _anthropic_proxy.py (~1K LOC: get_anthropic_client + ...) with explicit .py extensions.
  • Table row 38: _worktree_routes/_worktree_routes.py.
  • Table row 39: _anthropic_proxy/_anthropic_proxy.py.

The legend on lines 30–31 (trailing / = sub-sub-package, .py = flat module) is unchanged, so prose + table + legend + architect plan are all in agreement.

Scope is appropriately tight: only gateway/CLAUDE.md is touched (5 insertions, 4 deletions); no other files affected by this commit. python3 scripts/check-file-sizes.py exits 0.

No new issues

Examined the full delta. No new findings — the change is a surgical fix to the prior blocking issue with no scope creep.

Posting as --comment since this is a self-authored bot PR. From my read, this PR is ready to merge.

— Authored by egg

@james-in-a-box

This comment has been minimized.

@james-in-a-box

This comment has been minimized.

@james-in-a-box

Copy link
Copy Markdown
Contributor Author
egg is addressing review feedback...

@james-in-a-box

Copy link
Copy Markdown
Contributor Author

Both reviews against the current HEAD (81c5b8a) come up clean — nothing actionable to address in this pass.

Per-item disposition

Review Verdict Disposition
agent-mode-design @ 81c5b8a APPROVED — "No agent-mode design concerns" disagree (no actionable item raised — review explicitly approves)
review @ 81c5b8a COMMENTED — "No new issues … From my read, this PR is ready to merge" disagree (no actionable item raised — review explicitly states ready to merge)

Prior review rounds already resolved:

  • All four non-blocking findings from the c789c2c review → fixed in 6615c10
  • The one re-opened blocking finding (gateway/CLAUDE.md slice-11 misclassification) from the 6615c10 review → fixed in 81c5b8a

No new commit is needed for this run — there is no reviewer-requested change outstanding against HEAD.

— Authored by egg

@james-in-a-box

Copy link
Copy Markdown
Contributor Author

egg feedback addressed. View run logs

10 previous review(s) hidden.

@jwbron

jwbron commented Jun 11, 2026

Copy link
Copy Markdown
Owner

Closing unmerged. This slice's seam tables and retags encode the 28-slice plan from the issue-2817 pipeline, which failed in the implement phase on 2026-05-31; main has since invalidated that plan (#2993 / PR #2995 deleted two target files outright, two new files joined the allowlist, and the allowlist comment blocks this PR rewrites no longer exist in that form). Merging would write a stale map into four CLAUDE.md files. The program continues under #3111 — a fresh pipeline's scaffolding slice will redo this against current state.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant