docs: Update contract identifiers [doc-updater] - #630
Conversation
Clarify that contract files use issue numbers for issue-mode pipelines and pipeline IDs for local-mode pipelines, reflecting the fix in #626. Updated: - README.md: Contract path description - docs/guides/sdlc-pipeline.md: Contract storage paths - docs/architecture/orchestrator.md: Worktree artifact paths - docs/adr/implemented/ADR-SDLC-Pipeline.md: Contract state diagram Authored-by: egg
There was a problem hiding this comment.
No agent-mode design concerns. This is a documentation-only PR updating contract path references from {issue} to {identifier} to reflect the dual-mode (issue-mode / local-mode) contract lookup logic from #626. No agent workflows, prompts, or pipelines are affected.
— Authored by egg
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Review: docs: Update contract identifiers [doc-updater]
Verdict: Request changes (posted as comment — self-authored PR)
Issue: Incomplete update — {issue} references left behind
The PR updates the contract path from {issue} to {identifier} in four places, but misses several other occurrences in the same files that should also be updated for consistency. PR #626 introduced contract_key which affects all artifact paths (not just contracts), since local-mode pipelines use pipeline_id as the prefix for drafts, reviews, and contracts alike (see _verdict_path_for_type and _get_draft_path in orchestrator/routes/pipelines.py:827-872).
Missed references in docs/guides/sdlc-pipeline.md:
Lines 231, 242-254 still use {issue} in the file structure diagram:
.egg-state/
├── contracts/{issue}.json # ← should be {identifier}
├── drafts/
│ ├── {issue}-analysis.md # ← should be {identifier}
│ └── {issue}-plan.md # ← should be {identifier}
└── reviews/
├── {issue}-refine-review.json # ← should be {identifier}
...
Line 231 also references {issue} in prose:
writes its output to
.egg-state/drafts/{issue}-{analysis|plan}.md
Missed references in docs/architecture/orchestrator.md:
Lines 53-55 still use {issue} for drafts and reviews:
- `.egg-state/drafts/{issue}-analysis.md`
- `.egg-state/drafts/{issue}-{phase}.md`
- `.egg-state/reviews/{issue}-{phase}-{reviewer_type}-review.json`
These should either be updated to {identifier} or have parenthetical clarification added, consistent with the contract path on line 52 which now reads {identifier}.
Missed references in docs/adr/implemented/ADR-SDLC-Pipeline.md:
Lines 89 and 226 reference GET /api/v1/contract/{issue} — these should also be updated if the API now accepts pipeline IDs.
Minor: sdlc-pipeline.md line 663 accuracy
The populate-contract-tasks.py description on line 663 now reads:
Writes phases, tasks, and PR metadata into
.egg-state/contracts/{identifier}.json
This is technically correct for the file path, but the script itself (action/populate-contract-tasks.py) takes ISSUE_NUMBER as env var and only operates in issue-mode context (it fetches the plan from a GitHub issue). The {identifier} framing implies it works for local-mode too, which it does not. Either note this is issue-mode only, or leave the original {issue-number} here since the script is exclusively issue-mode.
Summary
The contract path updates are correct where applied, but the PR leaves the documentation in an inconsistent state — {identifier} for contracts, {issue} for drafts and reviews — when the actual code uses the same identifier logic for all artifact types. Either update all artifact path references consistently, or add a note explaining that drafts/reviews also use the pipeline identifier in local mode.
— Authored by egg
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Address review feedback on PR #630 by updating all remaining {issue} references to {identifier} in drafts and reviews paths across sdlc-pipeline.md, orchestrator.md, and ADR-SDLC-Pipeline.md. Also clarify that populate-contract-tasks.py is issue-mode only and revert its path back to {issue-number} since the script exclusively operates in issue-mode context.
There was a problem hiding this comment.
Agreed. All feedback addressed in b47b12e:
sdlc-pipeline.md: Updated all{issue}→{identifier}in the file structure diagram (lines 242-254), prose on line 231, and review cycle description.orchestrator.md: Updated draft and review paths on lines 53-55 to{identifier}.ADR-SDLC-Pipeline.md: UpdatedGET /api/v1/contract/{identifier}in both the diagram (line 89) and endpoint table (line 226).sdlc-pipeline.mdline 663: Reverted path back to{issue-number}and added "(issue-mode only)" clarifier, sincepopulate-contract-tasks.pyexclusively operates in issue-mode context.
— Authored by egg
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
No agent-mode design concerns in the delta (b47b12e). Changes since last review are documentation-only updates completing the {issue} → {identifier} path references across ADR, architecture, and guide docs. No agent workflows, prompts, or pipelines are affected.
— Authored by egg
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Re-review: docs: Update contract identifiers [doc-updater]
All four items from the previous review have been addressed correctly:
sdlc-pipeline.mdfile structure diagram — All{issue}→{identifier}in the tree, prose, and review cycle section. ✓orchestrator.mdartifact paths — Draft and review paths updated to{identifier}with clarifying parenthetical. ✓ADR-SDLC-Pipeline.mdAPI references — Updated in both diagram and endpoint table. ✓sdlc-pipeline.mdpopulate-contract-tasks — Reverted to{issue-number}with "(issue-mode only)" clarifier. ✓
No remaining {issue} references in the updated files.
Issue: ADR API endpoint is now less accurate than before
The ADR diagram (line 89) and endpoint table (line 226) were changed from GET /api/v1/contract/{issue} to GET /api/v1/contract/{identifier}. However, the actual route in gateway/contract_api.py:171 is:
@contract_bp.route("/<int:issue_number>", methods=["GET"])
def get_contract(issue_number: int) -> ...This endpoint only accepts integers (issue numbers). It does not accept pipeline IDs. The original {issue} was more accurate for this specific reference. Changing it to {identifier} implies the REST API supports local-mode pipeline IDs, which it does not.
Suggested fix for ADR-SDLC-Pipeline.md: Revert the API endpoint references back to {issue} (or {issue_number}), since these describe the actual REST API route, not the internal artifact path convention. The contract state diagram box (line 76) correctly uses {identifier} since that describes the file path, not the API.
This is a minor accuracy issue — readers relying on this ADR to understand the API contract would be misled about what the GET endpoint accepts. Not blocking, but worth fixing.
— Authored by egg
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
The GET /api/v1/contract/ route only accepts integers (issue numbers),
not pipeline IDs. Revert the API endpoint references in the diagram and
endpoint table back to {issue_number} for accuracy, while keeping the
file path reference as {identifier}.
There was a problem hiding this comment.
Agreed. Fixed in 4b2c39e — reverted both ADR API endpoint references (diagram line 89 and endpoint table line 226) from {identifier} back to {issue_number}, since the actual route in gateway/contract_api.py:171 only accepts integers. The file path reference on line 76 remains {identifier} as that correctly describes the artifact path convention.
— Authored by egg
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
No agent-mode design concerns in the delta (b47b12e → 4b2c39e). The only change since last review reverts the ADR API endpoint references from {identifier} back to {issue_number}, correctly reflecting that the REST route only accepts integers. No agent workflows, prompts, or pipelines are affected.
— Authored by egg
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Re-review: docs: Update contract identifiers [doc-updater]
Verdict: Approve
Previous feedback addressed
The suggestion from the last review — revert ADR API endpoint references from {identifier} back to {issue_number} — has been correctly addressed in 4b2c39e:
- ADR diagram (line 89):
GET /api/v1/contract/{issue_number}— matches the actual route/<int:issue_number>ingateway/contract_api.py:171. - ADR endpoint table (line 226): Same revert applied consistently.
- ADR contract state box (line 73): Correctly left as
{identifier}since this describes the file path, not the API.
Full PR state verified
Checked all four changed files at HEAD for consistency:
| Reference type | Placeholder used | Correct? |
|---|---|---|
| File paths (contracts, drafts, reviews) | {identifier} |
Yes — code uses contract_key which resolves to either issue number or pipeline ID |
| REST API endpoint | {issue_number} |
Yes — route is /<int:issue_number>, accepts integers only |
populate-contract-tasks.py path |
{issue-number} with "(issue-mode only)" |
Yes — script exclusively operates in issue-mode context |
No remaining {issue} references in any of the four changed files. The documentation now accurately reflects the dual-mode contract lookup from #626.
No issues found.
— Authored by egg
This comment has been minimized.
This comment has been minimized.
| egg is addressing review feedback... |
|
egg feedback addressed. View run logs 10 previous review(s) hidden. |
Update documentation to reflect changes from ea9b710 (Fix contract lookup for local-mode pipelines #626).
The contract lookup logic now supports both issue-mode and local-mode pipelines:
This PR updates documentation to clarify that contract files are stored at
.egg-state/contracts/{identifier}.jsonwhere the identifier depends on the pipeline mode.Changes:
Triggered by: ea9b710
Authored-by: egg