fix(describe): drop the blank card and keep diagrams left-to-right - #229
Merged
Merged
Conversation
Two regressions from the pr-agent 0.45.0 upgrade, both from new upstream defaults I did not pin, and neither reported as an error by anything. 0.45.0 stamps `<!-- pr-agent-generated -->` at the top of its output. That landed in the leading title-less section, whose body therefore counted as non-empty -- but an HTML comment renders to nothing, so every /describe result grew a card that looked blank. trimNoise already discarded blank lines, horizontal rules and the internal branch-name leak; it now also discards a line that is only an HTML comment, which makes the existing "no title and no content" rule cover this. 0.45.0 also added pr_diagram_direction='adaptive', which draws any chain past a few nodes top-down. Pinned to LR: upstream tunes that heuristic for a full-width page, while the diagram is read here in a narrow chat column, where a tall diagram pushes the rest of the result off screen and a wide one scrolls sideways and can be opened in the preview view. The pinning list in the design doc now separates the two ways an upstream default can be wrong here -- one adds output that parses as a bogus finding, the other changes presentation for a surface upstream cannot know about. The upgrade review only applied the first test, which is why the diagram default slipped through. 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
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.
Two regressions from the pr-agent 0.45.0 upgrade (#225). Both come from new upstream defaults I failed to pin, and neither is reported as an error by anything — they were found by looking at a real result.
1. Every
/describeresult grew a blank card0.45.0 stamps
<!-- pr-agent-generated -->at the top of its output (0.39.0 did not). That lands in the leading, title-less section, whose body therefore counts as non-empty — but an HTML comment renders to nothing, so the card came out blank.Reproduced from the reported output: 6 findings parsed, the first being
key=general,title=null, body exactly that comment.trimNoisealready discarded blank lines, horizontal rules and the internal branch-name leak; it now also discards a line that is only an HTML comment, which lets the existing "no title and no content → skip" rule cover this. 6 findings → 5, the rest unchanged. Two regression tests, including the case where the comment sits next to real prose and the section must be kept.2. Diagrams turned top-down
0.45.0 added
pr_description.pr_diagram_direction='adaptive', which draws any chain longer thanpr_diagram_direction_threshold(5) nodes top-down. The reported diagram had 6.Pinned to
LR, verified against the installed runtime. Upstream tunes that heuristic for a full-width page; here the diagram is read in a narrow chat column, where a tall diagram pushes the rest of the result off screen, while a wide one scrolls sideways and can be opened in the preview view.What I got wrong in the upgrade review
#225 pinned three new defaults, using one test: does it append to the output we parse? That test is incomplete. An upstream default can be wrong here in two distinct ways:
The diagram default is the second kind, so it passed the only check I applied. 05-pragent-runtime.md now states both, so the next upgrade is screened against both.
Also worth noting for the next upgrade: neither defect surfaced in
lint/typecheck/test/build, nor in the runtime probes I ran for #225 (imports, patch application, config overrides). Both needed a real/describeresult to be looked at.Verification
lint/typecheck/test/buildpass; the parsing fix was verified by re-running the reported output through the parser, and the diagram pin by reading the setting back out of the embedded runtime under the override.🤖 Generated with Claude Code