Repository navigation
SDK output envelopes, format-aware budgets, and stable error contracts - #895
Conversation
Expose typed mutation receipts and per-command envelope contracts through the SDK and runtime discovery surface. Add format-aware token ceilings, package fallbacks, semantic error classes, and a stable/provisional compatibility ledger. Harden workspace snapshot failures, document executable consumer flows, preserve 100 percent coverage, and make reindex coverage deterministic.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @unbraind, your pull request is larger than the review limit of 150000 diff characters
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
@greptileai please review this cohesive SDK/CLI output-contract tranche at the exact current head. |
Reviewer's GuideIntroduces format-aware output budgets and envelope contracts for all commands, adds semantic error exit classes with a stability ledger, makes workspace snapshot failures typed CLI errors, provides a public mutation receipt parser and example script, fixes reindex semantic refresh leakage, and updates contracts/docs/tests to reflect the new SDK/CLI context contract surface. File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
@coderabbitai full review |
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR adds format-aware command output budgets, output-envelope and mutation-receipt contracts, classified error metadata with stability validation, and structured workspace-snapshot errors. It also updates SDK exports, documentation, generated fixtures, tests, and PM tracking records. ChangesSDK output contracts
Error catalog and workspace errors
Supporting records and documentation
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
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 |
✅ Action performedFull review finished. Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 59 minutes. |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/sdk/error-code-catalog.ts (1)
77-99: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winValidate
sourcesandemitting_commandsafter normalization.Lines 77-78 check the raw array lengths. Lines 88-99 then trim each value and drop empty strings. An input of
[" "]passes validation and normalizes to[]. The published contract then carries an emptysourcesoremitting_commandslist, which the new nonempty invariant is meant to prevent.Normalize first, then validate the normalized lists.
🔧 Proposed fix to validate normalized lists
seen.add(code); + const normalizeList = (values: readonly string[]): string[] => [ + ...new Set(values.map((value) => value.trim()).filter(Boolean)), + ].sort(); + const sources = normalizeList(declaration.sources); + const emittingCommands = normalizeList(declaration.emitting_commands); if ( ![1, 2, 3, 4, 5].includes(declaration.exit_code) || declaration.meaning.trim().length === 0 || declaration.recovery.trim().length === 0 || - declaration.sources.length === 0 || - declaration.emitting_commands.length === 0 || + sources.length === 0 || + emittingCommands.length === 0 || ERROR_CLASS_BY_EXIT_CODE.get(declaration.exit_code) !== declaration.class ) { throw new TypeError(`Invalid pm error code contract: ${code}`); } return Object.freeze({ ...declaration, code, meaning: declaration.meaning.trim(), recovery: declaration.recovery.trim(), - sources: [ - ...new Set( - declaration.sources.map((source) => source.trim()).filter(Boolean), - ), - ].sort(), - emitting_commands: [ - ...new Set( - declaration.emitting_commands - .map((command) => command.trim()) - .filter(Boolean), - ), - ].sort(), + sources, + emitting_commands: emittingCommands, });🤖 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/sdk/error-code-catalog.ts` around lines 77 - 99, Update the contract validation flow around the declaration normalization to validate the normalized sources and emitting_commands lists, not their raw input lengths. Trim, filter empty values, deduplicate, and sort both lists first, then reject the declaration if either resulting list is empty; preserve the existing TypeError and frozen return behavior.
🤖 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 @.agents/pm/features/pm-x4nn3z.toon:
- Around line 82-86: Add a focused PM test entry for
tests/integration/workspace-snapshot-cli.integration.spec.ts alongside the
existing tests in the tests block, using the project’s PM CLI test command, and
record its result. Keep the change scoped to this PM item and preserve the
existing verification entries.
In `@scripts/generate-error-code-catalog.mjs`:
- Around line 149-159: Update inferEmittingCommands to track sources that do not
match command, lifecycle, or query paths and include "*" alongside resolved
command roots when any such source exists; retain ["*"] for sources with no
resolved roots. Regenerate src/sdk/generated-error-code-catalog.ts using pnpm
contracts:errors:update so invalid_argument_value and missing_required_option
reflect the wildcard origin.
- Around line 231-256: In main, defer bootstrapped stability-ledger creation
until after the rows mapping with renderCatalogRow completes successfully.
Preserve the missing-ledger --check error and stability validation, but move the
mkdir/writeFile block out of the stableCodes initialization path so render
failures cannot persist discovered codes; retain the generated ledger contents
and subsequent catalog behavior.
- Around line 161-173: Update readStabilityLedger to treat only an ENOENT read
error as a missing ledger, returning null in that case; rethrow all other
filesystem errors so main cannot overwrite the stability record after an
unreadable ledger failure.
In `@src/sdk/output-contracts.ts`:
- Around line 214-216: Update the parseMutationReceipt function signature to
remove the redundant string | unknown union. Use overloads to explicitly expose
the string JSON-parsing form alongside the unknown-value form, while preserving
the existing PmMutationReceipt return type and implementation behavior.
- Around line 71-102: Update the command classification in MUTATION_COMMANDS to
include all remaining single-item mutation commands, including start-task,
close-task, pause-task, and focus, so they receive mutation_receipt envelopes
with their top-level id and status. Add explicit envelope handling for bulk
mutations such as close-many and update-many instead of allowing them to use the
diagnostic fallback in the default branch around the contract generation logic.
Preserve the existing diagnostic and collection classifications for commands
that are not mutations.
In `@tests/fixtures/contracts/full.json`:
- Around line 8471-8524: Update the runtime contract generator around
resolvePmCommandOutputBudget and resolvePmCommandOutputEnvelope so each
generated subcommand row preserves its full command path, such as “extension
install”, instead of the root token. Ensure command_output_contracts emits
distinct command identifiers for every subcommand while retaining the declared
budget and envelope values, then regenerate the fixture.
- Around line 14892-14949: Update the error catalog generation/source mappings
so unknown_error uses generic_failure with class generic_failure and exit code 1
instead of usage/2. Then standardize unknown_field, unknown_item_type,
unknown_status, unknown_field_projection, unknown_option, and unknown_subcommand
under one consistent class and exit-code pair, updating their source definitions
and regenerated full.json entries together.
- Around line 12601-12613: Restore meaningful, authored contract metadata for
the affected entries in the generated full.json fixture: update each meaning to
describe the actual error condition, preferably deriving it from the
corresponding source error message such as core/item/parse.ts, and provide
actionable recovery guidance specific to each class or code instead of the
shared generic sentence. Regenerate the fixture using the project’s existing
contract-generation workflow so runtime consumers receive the corrected values.
In `@tests/unit/docs-examples/sdk-contract-consumer.spec.ts`:
- Around line 28-34: Update all four node:child_process mock factories in this
spec to include an execFile vi.fn alongside the existing spawnSync mock. Keep
each factory’s current spawnSync behavior unchanged so imports that promisify
execFile do not receive undefined.
In `@tests/unit/scripts/generate-error-code-catalog.spec.ts`:
- Around line 66-81: Extend the test around main(root, []) with a second
declaration of new_create_failure in a non-command source, such as a
cross-cutting module. Update the assertions for emitting_commands to verify both
the create command root and the * root, covering multi-source inference while
preserving the existing stability and code assertions.
In `@tests/unit/sdk/agent-output-contracts.spec.ts`:
- Around line 92-107: Add a separate assertion in the “rejects invalid format
budgets and empty generated command paths” test that passes an invalid value for
the json entry of default_max_estimated_tokens_by_format to
definePmCommandOutputBudget and verifies the expected validation error
references that entry, while preserving the existing toon and
createPmCommandOutputBudget coverage.
---
Outside diff comments:
In `@src/sdk/error-code-catalog.ts`:
- Around line 77-99: Update the contract validation flow around the declaration
normalization to validate the normalized sources and emitting_commands lists,
not their raw input lengths. Trim, filter empty values, deduplicate, and sort
both lists first, then reject the declaration if either resulting list is empty;
preserve the existing TypeError and frozen return behavior.
🪄 Autofix
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: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d0cf7a32-0a60-441f-913b-2d090f4be11c
📒 Files selected for processing (42)
.agents/pm/features/pm-5t33or.toon.agents/pm/features/pm-x4nn3z.toon.agents/pm/history/pm-5t33or.jsonl.agents/pm/history/pm-ay3l0p.jsonl.agents/pm/history/pm-cflhoj.jsonl.agents/pm/history/pm-srns.jsonl.agents/pm/history/pm-x4nn3z.jsonl.agents/pm/issues/pm-ay3l0p.toon.agents/pm/issues/pm-cflhoj.toon.agents/pm/issues/pm-srns.toonCHANGELOG.mddocs/README.mddocs/SCRIPTING.mddocs/SDK.mddocs/SELF_DESCRIBING_CONTEXT_CONTRACTS.mddocs/examples/sdk-contract-consumer/README.mddocs/examples/sdk-contract-consumer/package.jsondocs/examples/sdk-contract-consumer/parse-receipt.mjsscripts/agent-token-surface-baseline.jsonscripts/error-code-stability.jsonscripts/generate-error-code-catalog.mjssdk/public-surface.jsonsrc/sdk/cli-contracts.tssrc/sdk/cli-contracts/agent-output-contracts.tssrc/sdk/cli-contracts/runtime-contracts.tssrc/sdk/cli-program.tssrc/sdk/contracts.tssrc/sdk/error-code-catalog.tssrc/sdk/generated-error-code-catalog.tssrc/sdk/index.tssrc/sdk/output-contracts.tssrc/sdk/workspace-snapshot.tstests/fixtures/contracts/full.jsontests/integration/workspace-snapshot-cli.integration.spec.tstests/types/sdk-subpath-entrypoints.test-d.tstests/unit/commands/reindex-command.spec.tstests/unit/docs-examples/sdk-contract-consumer.spec.tstests/unit/scripts/generate-error-code-catalog.spec.tstests/unit/sdk/agent-output-contracts.spec.tstests/unit/sdk/error-code-catalog.spec.tstests/unit/sdk/output-contracts.spec.tstests/unit/sdk/sdk-subpath-entrypoints.spec.ts
Preserve normalized subcommand paths in envelope and token-budget lookup, classify single and bulk mutation shapes explicitly, and expose unambiguous mutation receipt overloads. Make error-catalog generation fail closed for unreadable ledgers, publish the ledger only after successful rendering, retain cross-cutting emitter provenance, and align unknown_error with generic failure exit semantics. Add review-driven regressions, refresh generated SDK/contracts/token surfaces, clarify scripting guidance, and record exact 100/100/100/100 plus npm/Bun consumer evidence in pm-srns, pm-5t33or, and pm-x4nn3z.
|
Review revision pushed at @coderabbitai full review |
|
✅ Action performedFull review finished. Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 3 minutes. |
|
Gemini notice acknowledged: the consumer reviewer is sunset, so it supplied no code-review evidence for this PR. |
|
Sourcery guide acknowledged; its file-level and issue mapping was useful for review orientation. |
|
CodeRabbit summary acknowledged. All 12 actionable findings received an inline disposition, reaction, regression evidence, and review-fix commit where applicable. |
|
CodeRabbit command acknowledgement noted; the resulting full first-head review was processed completely. |
|
CodSpeed report acknowledged. Its own report flags differing runtime environments; the exact-head hosted benchmark job passed, so no baseline was weakened. |
|
Codecov report acknowledged: patch coverage and the four-shard aggregate coverage gate pass on exact head. |
|
CodeRabbit second-review acknowledgement noted. The provider subsequently reported the new-head review as rate limited; this is recorded as a provider limitation, while all first-round findings were resolved and exact-head gates are green. |
|
Sourcery review-limit response acknowledged. It supplied no actionable review because the cohesive PR exceeds the provider limit; the separate Sourcery guide was still useful. |
|
CodeRabbit review acknowledged. All 12 findings were inspected, voted, answered inline, and either implemented with regression proof or dispositioned with the preserved semantic boundary. |
Normalize error sources and emitting command roots before enforcing non-empty catalog invariants, so whitespace-only declarations cannot publish empty provenance. Add regression coverage and record exact full-suite coverage evidence on pm-x4nn3z.
|
Outside-diff CodeRabbit finding implemented in @coderabbitai full review |
|
✅ Action performedFull review finished. Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 59 minutes. |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/unit/sdk/error-code-catalog.spec.ts (1)
36-41: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAssert the new error metadata in
resolvePmErrorCodeContract.The fixture includes
classandemitting_commands, but the assertion checks onlyexit_codeandstability. A resolver regression that drops either field would pass this test.Suggested assertion
).toMatchObject({ exit_code: 2, stability: "stable", + class: "usage", + emitting_commands: ["help"], });🤖 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 `@tests/unit/sdk/error-code-catalog.spec.ts` around lines 36 - 41, Extend the assertion for resolvePmErrorCodeContract("unknown_command", catalog) to also verify the fixture’s class and emitting_commands metadata, preserving the existing exit_code and stability checks.
🤖 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 @.agents/pm/features/pm-x4nn3z.toon:
- Around line 87-90: Remove the duplicate generate-error-code-catalog test
command from the PM metadata while retaining one canonical entry. Apply the
change through the PM CLI, then regenerate the derived projection; do not edit
the .toon or append-only history file directly.
---
Outside diff comments:
In `@tests/unit/sdk/error-code-catalog.spec.ts`:
- Around line 36-41: Extend the assertion for
resolvePmErrorCodeContract("unknown_command", catalog) to also verify the
fixture’s class and emitting_commands metadata, preserving the existing
exit_code and stability checks.
🪄 Autofix
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: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 7003fa3a-7be1-419e-9cb4-576d63008253
📒 Files selected for processing (20)
.agents/pm/features/pm-5t33or.toon.agents/pm/features/pm-x4nn3z.toon.agents/pm/history/pm-5t33or.jsonl.agents/pm/history/pm-srns.jsonl.agents/pm/history/pm-x4nn3z.jsonl.agents/pm/issues/pm-srns.toondocs/SCRIPTING.mdscripts/agent-token-surface-baseline.jsonscripts/generate-error-code-catalog.mjssdk/public-surface.jsonsrc/sdk/cli-contracts/agent-output-contracts.tssrc/sdk/error-code-catalog.tssrc/sdk/generated-error-code-catalog.tssrc/sdk/output-contracts.tstests/fixtures/contracts/full.jsontests/unit/docs-examples/sdk-contract-consumer.spec.tstests/unit/scripts/generate-error-code-catalog.spec.tstests/unit/sdk/agent-output-contracts.spec.tstests/unit/sdk/error-code-catalog.spec.tstests/unit/sdk/output-contracts.spec.ts
Assert the complete stable usage contract for resolved error codes and normalize the canonical PM evidence to three non-overlapping linked test gates.
|
@greptileai please review the exact final head 37b55c6 after the review corrections.\n\n@coderabbitai full review\n\n@sourcery-ai review |
|
Acknowledged CodeRabbit completion/rate-limit artifact IC_kwDORSgRDc8AAAABNQoCzw. Its two actionable findings were useful and are implemented at exact head 37b55c6; the provider limit is recorded as a review-availability limitation, not approval. |
|
Acknowledged CodeRabbit review PRR_kwDORSgRDc8AAAABIaEGDA. Both findings are implemented at exact head 37b55c6: catalog resolution now asserts class and emitting command ownership, and redundant PM-linked test commands were removed through pm. |
Rate Limit Exceeded
|
|
Acknowledged and up-voted all 12 CodeRabbit confirmation replies and their associated review containers. Each confirmation maps to an individually answered, resolved finding on the reviewed source head; no additional action was requested by those confirmation artifacts. |
|
Acknowledged empty CodeRabbit review container PRR_kwDORSgRDc8AAAABIaF3qQ associated with the exact-head rate-limit response. No finding was supplied; no approval is inferred. |
|
Acknowledged CodeRabbit exact-head rate-limit artifact IC_kwDORSgRDc8AAAABNQvN6w. It supplies no review finding; the provider limit is a documented availability limitation, while exact-head repository checks and the earlier substantive review remain separate evidence. |
|
Final artifact acknowledgement: down-voted CodeRabbit rate-limit reply PRRC_kwDORSgRDc7dg7yx and its empty review container PRR_kwDORSgRDc8AAAABIaHn0Q. They contain no findings; their parent thread is resolved because the underlying PM test duplication is fixed and verified. No additional inline reply is being sent because it would only generate another provider rate-limit artifact. |
Outcome
Delivers a cohesive SDK/CLI context-contract tranche: typed output envelopes and mutation receipts, format-aware command budgets, classified/stability-gated error metadata, typed workspace snapshot failures, and deterministic offline reindex coverage. This directly resolves the undocumented mutation/query envelope defect reported in #888 while retaining broader follow-on scopes as open PM features.
Closes #888.
PM lineage
What changed
Verification
Explicit limitations
Summary by Sourcery
Introduce typed output envelope and mutation receipt contracts across the SDK/CLI, enrich error metadata with semantic classes, stability gating, and emitting command ownership, and harden workspace snapshot flows and offline reindex behavior.
New Features:
Bug Fixes:
Enhancements:
Documentation:
Tests:
Chores:
Summary by cubic
Adds typed output envelopes and flat mutation receipts to the SDK/CLI, plus format-aware TOON/JSON budgets and a stability-gated error catalog with exit classes. Hardens contract discovery (normalized subcommand paths, explicit single/bulk mutations), clarifies
--json, validates normalized error provenance, and makes reindex coverage deterministic. Closes #888.New Features
PM_COMMAND_OUTPUT_ENVELOPE_CONTRACTS;parseMutationReceipt/isPmMutationReceiptwith unambiguous overloads.PM_COMMAND_OUTPUT_BUDGET_CONTRACTS; helperscreatePmCommandOutputBudgetandinferPmOutputBudgetClass.PM_ERROR_CODE_EXIT_CLASS_CONTRACTS, emitting commands, and a reviewed stability ledger.Bug Fixes
unknown_errormaps to exit 1; adds tests that lock resolution semantics and usage-class exits.--jsonhelp clarifies receipts vs envelopes; workspace snapshot emits typed usage errors; reindex coverage is deterministic and under 30s.Written for commit 37b55c6. Summary will update on new commits.