docs: trim the User Steering section to what a reader needs - #307
Merged
Conversation
The section was 133 lines — three times the next largest in the file, and 12% of it, for one message type among many. Most of the excess was the mechanics of the two-pass suppression budget, which are now commented at the code that implements them: the reason each part of the `(session, version, prompt-text)` key is needed, how a `remove`'s version is inferred, why the pre-pass shares the factory's predicate rather than mirroring it, what the imbalance warning does and does not claim, and which prompt shapes can be paired. Those comments were added alongside the code they describe, several of them recently, so the doc had become a second copy that loses on drift — the comment is next to the mechanism and moves with it. What stays is what a reader of a message-type reference wants: the two record shapes and their eras, that one delivery writes both and only one card renders, that nothing is dropped when pairing fails, the prompt shapes, and the measured null-content boundary with its two sampling limits. The content moved rather than vanished. 78 lines, from 133.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughExpanded User Steering documentation to cover paired and unmatched records, prompt normalization, transformer behavior, fallback warnings, legacy null-content removes, version observations, and sampling limitations. ChangesSteering behavior documentation
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@dev-docs/messages.md`:
- Around line 286-290: Update the warning-behavior description in the pairing
documentation to state that legacy remove warnings occur only when the (session,
version) exists in qc_versions; do not claim that every unmatched legacy remove
logs a warning. Retain the statement that rendered but non-pairable cards leave
remove visible and emit a warning.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
"Both paths log a warning" was too broad. The two sites are asymmetric: * a non-pairable prompt shape warns unconditionally (`attachment_factory.queued_command_prompt_items`); * an unmatched legacy `remove` warns only when its `(session, version)` is in `qc_versions` — i.e. only where that version did produce a counted card — and then at most once per key. So an ordinary legacy-only transcript renders its `remove` as legacy steering in silence, which is correct: with no card there is nothing for it to be imbalanced against. The old sentence would have had a reader hunting for a warning that cannot fire. Raised in review on #307. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
What this does
Trims
### User Steering (Queue Remove / queued_command)indev-docs/messages.mdfrom 132 lines to 77, keeping every measured claim byte-identical and cutting
the investigation narrative that accumulated while the behaviour was being worked
out.
At 132 lines it was the largest behavioural section in the file — the next is
### Tool Result Rendering Wrapperat 87 — and second overall only to### Tool Use Message Structure(294 lines), which is a reference table coveringevery tool rather than one message type.
dev-docs/is as-built reference: thecode is the authority, and prose earns its place by telling a reader something the
code cannot.
What is preserved, deliberately byte-for-byte
Every measured statement, because none of it is re-derivable from the code — it
came from counting real archives:
(2.1.202, 2.1.205]in which the behaviour changed;the two observations that narrow the interval to that shape;
not establish;
>=~2.1.187claim was backwards.That one is kept precisely because it is a correction: without it the next
reader re-derives the same wrong direction from the same data.
What went is the reasoning that produced those numbers, the alternatives
considered and rejected, and the step-by-step account of how the pairing was
reverse-engineered. Useful while the behaviour was unknown; noise once it is
documented.
Why this closes #294
#294 reported a warning from a real archive:
The pairing was not violated. Image-bearing steering prompts were being
dropped, so the paired legacy
removerendered its text alone and lookedorphaned — and the warning's own diagnosis pointed at the wrong thing, which is
what made it hard to see. Two changes resolved it:
removing the cause;
[Image #N]placeholders from therecorded
imagePasteIdsrather than by block position — so the images landwhere their references actually point.
The detector itself is deliberately retained and scoped, not deleted: it still
fires for a genuine pairing violation, and tests pin both that it fires when the
pairing really breaks and that it stays silent otherwise. What changed is that it
no longer accuses the archive of something our own renderer was doing.
This PR is the documentation tail of that arc — the section had grown while the
behaviour was being established, and it is the last piece still carrying the
investigation. Neither #305 nor #306 used a closing keyword, so the issue stayed
open after the code was fixed; it closes here.
Closes #294
Summary by CodeRabbit