fix(status): order artifacts by the schema, not the alphabet - #1465
Merged
Conversation
Artifacts that become ready at the same time were sorted alphabetically,
so spec-driven's `specs` and `design` - both requiring only `proposal` -
came back as design first. `openspec status` listed design above specs
and `nextSteps` pointed at design, sending agents to write design.md
before any spec existed. That contradicts the schema's own description
(proposal -> specs -> design -> tasks), the design instruction ("reference
the specs for requirements"), the workflow docs, and the schema `openspec
schema init` scaffolds (where design requires specs).
Break ties by the order the schema declares its artifacts instead. The
dependency edges are untouched, so nothing newly blocks and no artifact
becomes mandatory - only the order of equally-ready artifacts changes, and
it now follows the sequence the schema author wrote, for custom schemas
too.
Closes #692
Closes #695
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughArtifact ordering now follows schema declaration order across graph operations, instruction generation, status output, tests, and documentation. Ready artifacts, build sequences, unlocked artifacts, and blocked dependencies use the declared sequence instead of alphabetical sorting. ChangesArtifact ordering
Estimated code review effort: 3 (Moderate) | ~25 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Deploying openspec-docs with
|
| Latest commit: |
e19b7de
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://b60990c6.openspec-docs.pages.dev |
| Branch Preview URL: | https://fix-artifact-order-declarati.openspec-docs.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/core/artifact-graph/graph.ts (1)
109-129: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRe-sort the entire ready queue after adding newly ready artifacts.
Sorting only
newlyReadyleaves existing queue entries ahead of earlier-declared artifacts. For a schema ordered[root, child, laterRoot], wherechildrequiresroot, the build order becomesroot, laterRoot, childeven thoughchildandlaterRootare both ready afterrootandchildis declared first.Append the new entries, then sort the full queue (or use a declaration-order priority queue), and add a regression test for this interleaving case.
Suggested fix
- queue.push(...newlyReady.sort(this.compareByDeclarationOrder)); + queue.push(...newlyReady); + queue.sort(this.compareByDeclarationOrder);As per coding guidelines, run the focused test with
pnpm exec vitest run test/core/artifact-graph/graph.test.ts.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/core/artifact-graph/graph.ts` around lines 109 - 129, Update the topological ordering loop to sort the entire ready queue after appending newly ready artifacts, using compareByDeclarationOrder, so existing and newly added entries are globally declaration-ordered. Add a regression test in the artifact graph tests covering [root, child, laterRoot] and asserting root, child, laterRoot order, then run the focused Vitest command specified in the review.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/core/artifact-graph/graph.ts`:
- Around line 109-129: Update the topological ordering loop to sort the entire
ready queue after appending newly ready artifacts, using
compareByDeclarationOrder, so existing and newly added entries are globally
declaration-ordered. Add a regression test in the artifact graph tests covering
[root, child, laterRoot] and asserting root, child, laterRoot order, then run
the focused Vitest command specified in the review.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 60982aaa-38c6-4d00-8b47-48f08fb6c9ee
📒 Files selected for processing (6)
docs/cli.mddocs/customization.mdsrc/core/artifact-graph/graph.tstest/commands/artifact-workflow.test.tstest/core/artifact-graph/graph.test.tstest/core/artifact-graph/workflow.integration.test.ts
CodeRabbit caught it: sorting only the newly ready artifacts left an already-queued artifact ahead of one declared earlier. For [root, child, laterRoot] where child requires root, the build order came out root -> laterRoot -> child even though child is declared first and both are ready after root. Sort the full queue after each push. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adversarial review found `unlocks` was left alphabetical while build order,
ready lists and blocked lists moved to declaration order, so `openspec
instructions proposal` said "enables: design, specs" while `openspec status`
listed specs first - the one field whose job is naming what comes next
disagreed with everything else. getAllArtifacts() already yields declaration
order, so the stray sort is simply dropped.
Also make compareByDeclarationOrder a method rather than an arrow-valued
field: the field added an own enumerable function property that made
ArtifactGraph fail structuredClone.
Docs and specs updated for the new guarantee:
- openspec/specs/{artifact-graph,cli-artifact-workflow,instruction-loader}
- docs/agent-contract.md: status --json and instructions --json ordering
- docs/opsx.md: the status sample's missingDeps was missing design
- changeset
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…d lines The sample said tasks was blocked by specs alone and that creating specs made tasks available; tasks needs design too. Same class of inaccuracy as the status samples this branch already corrected. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/opsx.md`:
- Line 507: Update the JSON example’s tasks entry to include the required
"status": "blocked" field alongside its existing properties, preserving the
documented status shape for every artifact.
In `@openspec/specs/artifact-graph/spec.md`:
- Around line 91-94: Update the “Ready artifacts ordered by declaration”
scenario in the artifact graph specification to use the standard wording “the
artifact the schema recommends writing next” instead of “the artifact the schema
wants written next.”
In `@openspec/specs/cli-artifact-workflow/spec.md`:
- Around line 43-49: Update the status-ordering documentation at
openspec/specs/cli-artifact-workflow/spec.md lines 43-49 and the JSON contract
at docs/agent-contract.md line 58 to describe topological ordering from
getBuildOrder(), using schema declaration order to break ties rather than
guaranteeing raw declaration order; preserve the stated ready-artifact and
missingDeps ordering behavior at both sites.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 44023c8d-385c-4e6f-aab3-01169d84bf80
📒 Files selected for processing (10)
.changeset/schema-declared-artifact-order.mddocs/agent-contract.mddocs/commands.mddocs/opsx.mdopenspec/specs/artifact-graph/spec.mdopenspec/specs/cli-artifact-workflow/spec.mdopenspec/specs/instruction-loader/spec.mdsrc/core/artifact-graph/graph.tssrc/core/artifact-graph/instruction-loader.tstest/core/artifact-graph/instruction-loader.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/core/artifact-graph/graph.ts
CodeRabbit was right that "artifacts appear in the order the schema declares them" over-claims: dependency order still wins, and declaration order only breaks ties. Proved with a schema that declares tasks, specs, proposal - status renders proposal, specs, tasks, not the declared order. Corrected in the cli-artifact-workflow spec, agent-contract.md, cli.md and the changeset. Also restores "status": "blocked" in the opsx.md status sample (split across two lines so the ASCII box still aligns) and uses "recommends writing next" in the artifact-graph spec. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
alfred-openspec
approved these changes
Jul 28, 2026
alfred-openspec
left a comment
alfred-openspec
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed at e19b7de. Declaration order now breaks only dependency ties across build order, ready artifacts, blocked dependencies, next steps, and unlocks; dependency edges and optionality are unchanged. 147 focused tests, build, lint, strict validation, the 2,296-test cross-platform suite, CodeQL, audit, dependency review, and release tracking all pass.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Status
Ready. An ordering fix with no gating change: no dependency edge was added or removed, so nothing newly blocks and no artifact becomes mandatory. Four adversarial reviews were run against it; everything they confirmed is fixed in this branch.
What was wrong
OpenSpec told agents to write
design.mdbefore any spec existed.specsanddesignboth require onlyproposal, so both become ready at the same moment. The graph broke that tie alphabetically, and "design" sorts before "specs". On a change with only a proposal:Everything else in OpenSpec said the opposite — and in two places the CLI already contradicted itself:
schemas/spec-driven/schema.yamldescriptionproposal → specs → design → tasksdesigninstructionopenspec schemas(reads declaration order)Artifacts: proposal → specs → design → tasksdocs/opsx.mdstatus samplespecsfirst, annotated◄── First readydocs/commands.mdcontinue transcript◆ specs (ready)above◆ design (ready), "Creating specs..."openspec schema initHow it was fixed
Ties are broken by the order the schema declares its artifacts (
src/core/artifact-graph/graph.ts). That coversgetBuildOrder,getNextArtifacts, andgetBlocked, soopenspec status,status --json,nextSteps, andblocked by:lists all agree.openspec instructions'unlocksfield was still alphabetical and is now consistent too.design.mdstays optional and still writable early — you just get a warning, exactly as before. Custom schemas get the same guarantee: list artifacts in the order you want them written.#695 and #1173 proposed adding
specstodesign.requiresinstead. That was deliberately not done: it would mark design blocked before specs exist, and drafting design early is legitimate. It also isn't needed to fix the reported behavior.Proof it works
unlocksorder, and thestatus --jsonartifact list.init → new change → proposal → specs → design → tasks → validate --strict → archivebehaves as before, with the spec merged intoopenspec/specs/and the change archived.list,show --json,schemas,templates --json,instructions apply --json, andinstructions archive --jsonare byte-identical tomain. Onlystatus,nextSteps, andunlocksmoved.requiresall behave exactly as onmain(schema validation rejects the invalid ones before the graph sees them). Perf of the per-iteration queue re-sort at 2000 artifacts: ~54 ms; real schemas have four.Review findings folded in
[root, child, laterRoot]gaveroot → laterRoot → child). The whole queue is re-sorted now, with a regression test.unlockswas still alphabetical, soinstructionsnamed artifacts in one order whilestatusrecommended another.ArtifactGraphremainsstructuredClone-able.Docs and specs updated
openspec/specs/artifact-graph/spec.mdopenspec/specs/cli-artifact-workflow/spec.mdstatuslists artifacts in declared order; first[ ]is the recommended next artifactopenspec/specs/instruction-loader/spec.mdunlocksuses the same orderdocs/agent-contract.mdstatus --jsonandinstructions --jsondocs/cli.mddocs/customization.mddocs/opsx.md,docs/commands.mdmissingDeps/ what a new artifact unlocks.changeset/No template or skill content changed, so no golden-hash regeneration.
Notes
design.mdexists but specs do not now renders[ ] specsabove[x] design. Progress counts, statuses, andnextStepsare all still correct.requiresedge) is a maintainer design call.🤖 Generated with Claude Code
Summary by CodeRabbit
statusoutput and recommended next steps now use this deterministic ordering for ties.artifactsandmissingDeps.