Repository navigation
Fix: quoted review headings start no block - #101
Merged
Merged
Conversation
Plan-lifecycle 0.17.0 split a review comment into blocks after it had
removed blockquote and list markers from each line. A passing block
that quoted an earlier round, such as
> ### Code review round 1
> Code-review: fixes-required
then opened a block of its own whose quoted verdict won, so the round
read as fixes-required (or repair for a plan review). A delta reviewer
that quotes old rounds could keep a loop from passing. Version 0.16.0
read only the first verdict line and did not have this problem.
The helper now starts a later block only at a Markdown heading line
that is not quoted and not a list item, indented by at most three
spaces. Kinds and verdicts are still read through emphasis, code
marks, quotes, and list markers. The contract states the rule, and the
parser spec covers a quoted heading, a list-item heading, and an
indented lens heading.
Refs #98
Code review of the quoted-heading fix found that the helper removed emphasis marks from the whole line before it looked for a heading, so a `*` list marker vanished and `* ### Code review round 1` with a quoted `* Code-review: fixes-required` still opened a block whose verdict won. The helper now requires `#` after at most three spaces on the raw line and reads only the title after the `#` markers through emphasis and code marks. The parser spec adds the star-bullet case and an emphasized heading title that must still start a block. Refs #98
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.
Fix: quoted review headings start no block
Plan-lifecycle 0.17.0 (#98, pull request #99) lets one review comment hold several blocks, one per lens. To find a later block, the helper looked for a review heading after it had removed blockquote, list, and emphasis marks from each line. A passing block that quoted an earlier round then opened a block of its own:
The quoted verdict won, so the round read as
fixes-required(orrepairfor a plan review). A delta reviewer that quotes old rounds could keep a review loop from passing. The failure was strict, never loose: a quoted heading could not turn a failure into a pass. Version 0.16.0 read only the first verdict line and did not have this problem.What changed
#after at most three spaces, not quoted and not a list item. Only the title after the#markers is read through emphasis and code marks (plugins/plan-lifecycle/skills/productivity/plan-manager/scripts/plan.mjs).plan-contract.md).Verification
fixes-requiredbefore the fix; the star-bullet case (* ### Code review round 1) still readfixes-requiredafter the first commit, which code review round 1 found.node scripts/tests/plan-cli.mjs: 66 of 66 passed, with cases for a quoted heading, a-and a*list-item heading, an indented lens heading, and an emphasized heading title.bun run test:coverage:plan.mjsline coverage 99.68%.bun scripts/ci.mjs: passed for the repo-wide checks and both plugins.After merge
Refs #98