Skip to content

docs: update artifact_spec docs for slice-2 [doc-updater] - #3151

Merged
jwbron merged 2 commits into
mainfrom
egg/doc-update-artifact-spec-slice2
Jun 13, 2026
Merged

docs: update artifact_spec docs for slice-2 [doc-updater]#3151
jwbron merged 2 commits into
mainfrom
egg/doc-update-artifact-spec-slice2

Conversation

@james-in-a-box

Copy link
Copy Markdown
Contributor

docs: Update artifact_spec docs for slice-2 landing

Update documentation to reflect changes from #3141 (slice-2 of #3077):

  • docs/development/STRUCTURE.md: add artifact_spec.py to the egg_contracts/ module listing — it introduces the declarative artifact registry (ArtifactSpec, specs_for, spec_by_name, resolve_artifact_path) that is the single source of truth for per-phase coordination artifact paths
  • docs/architecture/coordination-state.md: mark the two slice-2 rows as Shipped (were Pending), and bump the section heading from "as of slice-1" to "as of slice-2"

Triggered by: #3141

Authored-by: egg

Update docs for #3077 slice-2 (artifact_spec.py merged in #3141):
- STRUCTURE.md: add artifact_spec.py entry to egg_contracts listing
- coordination-state.md: mark slice-2 rows Shipped, bump heading to slice-2
@james-in-a-box

This comment has been minimized.

@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.

Approve. Docs-only PR; every factual claim verifies against the code on origin/main. Reviewed both changed files line-by-line and traced each assertion to its source.

Verification performed

shared/egg_contracts/artifact_spec.py — exists with the claimed surface. Confirmed present on origin/main (not just the worktree) and provides exactly the symbols the PR body cites:

  • class ArtifactSpec (artifact_spec.py:42, frozen @dataclass(slots=True))
  • spec_by_name (:198), specs_for (:214), resolve_artifact_path (:227)

The module docstring describes "per-phase coordination artifacts that producer roles commit and consumer roles read" with path_template, phase, producer_role, consumer_roles fields — so the new STRUCTURE.md line ("single source of truth for per-phase coordination artifact paths, producers, and consumers") is an accurate description, not aspirational.

shared/egg_contracts/tests/test_artifact_spec.py — exists (28 KB on origin/main). The "Shipped (slice-2)" flip for the test row is correct.

Slice-3+ rows correctly left as Pending. Spot-checked the two nearest-term pending claims so the table isn't understating progress:

  • slice-3 (handle_consensus_propose_signal generalisation): grep for specs_for / spec_by_name / resolve_artifact_path / artifact_spec across orchestrator/ returns nothing — the registry is not yet wired into propose-time validation. Pending is accurate.
  • slice-4 (gateway POST /api/v1/artifact/get + routes/artifacts.py): no matches in gateway/ or orchestrator/. Pending is accurate.

Heading bump consistent. "status as of slice-1" → "slice-2" matches the table: slice-2 is now the latest fully-shipped slice, slices 3–6 remain Pending. No slice-2 row is left as Pending.

STRUCTURE.md placement. The egg_contracts/ listing is a deliberately curated subset (it omits audit.py, decisions.py, feedback.py, hitl.py, etc.), consistent with the #3132/#3156 backfill. Inserting artifact_spec.py after plan_parser.py is editorial ordering, in line with the rest of the list — no issue.

Issues

None. No blocking or non-blocking concerns. The diff is minimal, the claims are true, and the pending/shipped boundary is drawn correctly.

— Authored by egg

@james-in-a-box

This comment has been minimized.

@james-in-a-box

This comment has been minimized.

…onflicts in coordination-state.md and STRUCTURE.md

main advanced #3077 from slice-2 to slice-6, so its slice-landing
status (heading + table rows) is a superset of this PR's slice-2
update — took main's side for both conflict blocks. Removed the
duplicate artifact_spec.py STRUCTURE.md entry left by the auto-merge
(kept main's alphabetically-sorted entry).
@james-in-a-box

Copy link
Copy Markdown
Contributor Author

Conflict Resolution Summary

Merged origin/main into this branch (merge, not rebase). Two files conflicted:

File Category Resolution
docs/architecture/coordination-state.md Semantic (temporal precedence) Took main's side for both conflict blocks
docs/development/STRUCTURE.md Additive (de-duplicated) Removed duplicate artifact_spec.py entry left by the auto-merge

Details

coordination-state.md — Two conflict blocks: the section heading (status as of slice-2 vs slice-6) and the slice-landing table rows (slices 3–6 as Pending vs Shipped). Since this PR opened, main advanced #3077 from slice-2 through slice-6 (via #3152 and follow-ups), so main's state is a strict superset of this PR's intent — the slice-2 "Shipped" rows this PR added are already present on main (non-conflicting lines), and main additionally marks slices 3–6 shipped. Took main's side for both blocks (temporal precedence: newer state supersedes).

STRUCTURE.md — Git auto-merged this file but produced two artifact_spec.py entries in the egg_contracts/ listing: this PR's (placed after plan_parser.py) and main's (placed after agent_recovery.py). Both describe the same file. Kept main's entry — it preserves alphabetical ordering (agent_recoveryartifact_specaudit) — and removed the PR's duplicate.

Please review

  • The net effect on coordination-state.md is that this PR no longer changes the file (its slice-2 update was already superseded by main). This is expected for a doc PR overtaken by later landings.

Note on local verification

make lint surfaced 3 pre-existing mypy errors in shared/egg_agent/client.py. That file is byte-identical to origin/main at the merged HEAD and was never touched by this branch — the errors exist on main independently and are out of scope for this docs-only merge. CI will run the full suite.

— 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.

Comment — merge resolution is correct, but the PR is now a no-op. Recommend closing as superseded rather than merging.

Re-reviewed the delta since my last review at f9466295. The only new commit is the merge 35a8440 resolving conflicts after origin/main advanced #3077 from slice-2 to slice-6.

The merge resolution is correct

I traced both conflict files independently and found no dropped content or defects:

  • coordination-state.md — taking main's side loses nothing. main is a genuine strict superset: the PR's slice-2 changes (heading slice-1slice-2, and the artifact_spec.py / test_artifact_spec.py rows flipped to Shipped (slice-2)) are all present on main, which has bumped the heading to slice-6 and additionally marks slices 3–6 Shipped (coordination-state.md:25,39-43).
  • STRUCTURE.md — the auto-merge produced two artifact_spec.py entries; the resolution correctly kept main's (alphabetical agent_recovery → artifact_spec → audit, line 321) and removed the PR's duplicate. HEAD has exactly one entry. Clean.

The author's Conflict Resolution Summary is accurate and transparent.

But the PR no longer changes anything

The merged HEAD tree (8483b75c…) is byte-identical to origin/main, and gh pr diff 3151 returns empty. The PR's net contribution to main is zero — every change it set out to make (the artifact_spec.py STRUCTURE entry, the slice-2 status flips) already landed on main via #3152 and the slice-3..6 follow-ups.

Merging this would add an empty merge commit to main's history conveying nothing. The correct disposition is to close the PR as superseded by the later slice landings. There is no code to change — this is an operator action, which is why this is a comment and not a request-for-changes.

(Author's note about pre-existing mypy errors in shared/egg_agent/client.py checks out — that file is byte-identical to main at the merged HEAD, since the whole tree is.)

— Authored by egg

@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

Feedback disposition

Thanks for the careful re-reviews. Verified state before responding: gh pr diff 3151 is empty and the PR HEAD tree (8483b75c…) is byte-identical to origin/main (git diff 35a8440 origin/main → no output). So there is no code, docs, or test change available to make in this PR.

Per-item:

  1. Review at f9466295 — APPROVED, no concerns. Nothing to address. acknowledged (no change requested)

  2. Review at 35a8440 — "merge resolution is correct, but the PR is now a no-op; recommend closing as superseded." disagree-style tag doesn't fit — I agree with the assessment. But there is no in-PR code change that addresses it: every change this PR set out to make (the artifact_spec.py STRUCTURE.md entry and the slice-2 status flips) already landed on main via docs: update coordination-state for #3077 slices 2-4 [doc-updater] #3152 and the slice-3..6 follow-ups, leaving a zero-diff branch. The recommended action — closing the PR as superseded — is an operator action I cannot take in code (I don't merge or close PRs). acknowledged (operator action — close as superseded; no code change possible)

No follow-up issue filed: there is no tracked work to carry forward — the intended changes are already on main. No commits pushed: an empty branch has nothing to commit.

Recommendation to the operator: close PR #3151 as superseded by the later slice landings rather than merging (a merge would add an empty merge commit conveying nothing).

— Authored by egg

@james-in-a-box

Copy link
Copy Markdown
Contributor Author

egg feedback addressed. View run logs

6 previous review(s) hidden.

@jwbron
jwbron merged commit 84f5de8 into main Jun 13, 2026
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant