fix: coerce mis-wrapped compress content args and teach format on rejection - #616
Open
kido5217 wants to merge 5 commits into
Open
fix: coerce mis-wrapped compress content args and teach format on rejection#616kido5217 wants to merge 5 commits into
kido5217 wants to merge 5 commits into
Conversation
…ection Some models emit compress 'content' as a plain summary string instead of the required array of entry objects (no array wrapper, no startId/endId). validateArgs then failed with the bare 'content is required and must be a non-empty array' error, so the model had to spend a full extra turn reformatting the call (10 such failures observed in recent opencode sessions, every one this shape). Root cause: model-side arg mis-wrap, not a validation bug. The compressed range is genuinely unknowable from a plain string, so it cannot be defaulted. The fix: - coerce unambiguous mis-wraps at runtime (a single entry object, or a JSON string encoding an entry / entry array) before validation - for the rest (plain strings), return a guiding error that shows the exact expected shape and where to find boundary IDs (mNNNN / bN, from <dcp-message-id> tags in context) Applies to both the range and message compress tools. Regression tests replay the captured failing payload plus the coercion cases.
coerceContentArray treated a "[]" string like an unparseable or plain string and threw "content must be a JSON array, not a plain string" — but the input IS a JSON array, just empty. A model reading that diagnosis gets no signal about the real problem (nothing to compress). An empty parsed array now throws the same "content is required and must be a non-empty array" error a raw [] gets, in both the range and message compress tools.
The range and message tools carried two copies of the same logic: the whole-args shape guard with a re-send example (normalizeRangeArgs / normalizeMessageArgs) and the per-field entry guards (isRangeEntry / isMessageEntry). Extract the shared shape into args.ts, which already owns the content coercion: - isStringFields: plain-object guard with named string fields, used by both entry guards (each now states its field list once) - normalizeCompressArgs: the whole-args guard plus content coercion, parameterized by entry guard, content noun, shape example, and the per-mode re-send guidance Behavior is unchanged; the per-mode guidance strings are preserved verbatim. Also document in args.ts that coerceContentArray leaves element-shape checks to each tool's validateArgs.
The literal 'content is required and must be a non-empty array' lived in four places: both throw sites in coerceContentArray (args.ts) and both validateArgs copies (range-utils.ts, message-utils.ts). It is a single model-facing message, so it now has one source of truth (NON_EMPTY_ARRAY_ERROR_MESSAGE in args.ts); wording changes are a one-place edit. Behaviour is byte-identical - the wording-pinning tests assert the literal and still pass.
The KISS-lens review of the branch judged the message-mode execute replay test unnecessary: execute() calls normalizeMessageArgs on its first line, so the test verifies call wiring, not behavior, and the plain-string rejection it replays is already pinned by the unit test directly above. The range-mode replay is kept because it embeds the real captured payload from opencode sessions. Behavior unchanged: 114/114 tests, typecheck, prettier, and build 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
1 participant
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.
Models sometimes pass
contentto the compress tool as a plain summary string instead of an array of entries. The bare validation error ("content is required and must be a non-empty array") gave no format hint, so the model paid a full extra turn to retry (10 such failures observed in opencode sessions, all this shape)."[]") gets the non-empty-array error instead of a misdiagnosisFollow-up commits address code-review findings: dedupe the arg guards/normalization into
lib/compress/args.ts, hoist the shared error message to a constant, drop a redundant test.Tests: 114/114; typecheck, prettier, and build all green.