fix: sync literal backslash URL preservation #26

jercik · 2026-10-03T20:23:58Z

jercik commented

2026-10-03 20:23:58 +00:00

Syncs the mirrored consumer introduced in #20 with canonical review #101 at ff94ddf: rewriting angle-bracket destinations must preserve the literal backslash in [a](<docs/a\ b.md>), retaining docs/a%5C%20b.md instead of docs/a%20b.md.

Canonical #101 remains an independent review dependency; this consumer PR targets main and does not establish the canonical PR's readiness.

Syncs the mirrored consumer introduced in [#20](https://code.j4k.dev/j4k-oss/review-wrapper/pulls/20) with [canonical review #101](https://code.j4k.dev/j4k/review/pulls/101) at [`ff94ddf`](https://code.j4k.dev/j4k/review/commit/ff94ddf36344f37f8539a2e5d353987791434d2a): rewriting angle-bracket destinations must preserve the literal backslash in `[a](<docs/a\ b.md>)`, retaining `docs/a%5C%20b.md` instead of `docs/a%20b.md`. Canonical #101 remains an independent review dependency; this consumer PR targets `main` and does not establish the canonical PR's readiness.

jercik added 1 commit

2026-10-03 20:23:58 +00:00

fix: sync literal backslash URL preservation

commit-msg / commitlint (pull_request) Successful in 18s

Details

Checks / quality-checks (pull_request) Successful in 33s

Details

Review / Review (pull_request_target) Successful in 7m54s

Details

3be32e1aea

forgejo-actions commented

2026-10-03 20:31:51 +00:00

Review 01M41Q08APM5XD2ZASJB6K2CVY — head 3be32e1aea064149a5fa858639ee38959c3b4e2d

Review — j4k-oss/review-wrapper @ `c032678d10`

Scope: diff against base tree af4a01b58937
Status: dispatched — coverage complete (4/4 slots terminal)
Facts: current review-wide projection

Computed under:

{
  "abandonment": "abandonment-v1",
  "anchor_recipe": 1,
  "batch_policy": "batch-v1",
  "coverage": "coverage-v3",
  "dispatch_policy": "dispatch-v2",
  "grounder_version": 1,
  "grounding_read_rule": "grounding-read-v1",
  "promotion_policy": "promotion-v1",
  "report": "report-v3",
  "tally": "tally-v1",
  "triage_settle": "triage-settle-v2"
}

Findings (1)

low — Describe destination conversion accurately in the regex comment

claim: 01M41Q7ZAE16K8X30SYDA4RGV7
anchor: src/markdown/neutralize-html.ts (snippet)

/** A backslash escape, or a character only an angle-bracket destination may hold. */

lens: writing-quality · arm: default
verdicts: 1 valid / 0 invalid / 0 uncertain
duplicates: 01M41QAFSAXXKTM8TW25VA1A0A (restated-sets)
disposition: none

I read the DESTINATION_UNSAFE declaration, plainDestination, and the new neutralize-html test for a backslash before whitespace. The comment says the second regex branch contains characters only an angle-bracket destination may hold. That branch now includes a bare backslash; the converter also emits escaped parentheses in a plain destination. The distinction a maintainer needs is which characters require conversion to preserve the destination, so the present explanation can lead them to remove the backslash case that this change fixes. Replace the comment with: 'Backslash escapes to preserve and characters this converter encodes or escapes when removing destination brackets.' This preserves the regex's purpose without claiming those characters are exclusive to bracketed destinations. The mismatch follows from the pattern, conversion function, and regression assertion; I did not verify renderer output independently.

Other claims

grounding-pending (0)
ungrounded (0)
rejected (0)
duplicate-of (1)
- 01M41QAFSAXXKTM8TW25VA1A0A high — DESTINATION_UNSAFE comment omits standalone backslashes from its match set → 01M41Q7ZAE16K8X30SYDA4RGV7
unadjudicated (0)

Coverage

Coverage pass: 01M41Q08D84CMFX59F5HAQHJ93
Accounting: complete
Slot health: healthy

lens	part	arm	unit status	runs	loss
general-bug	whole	default	no-claims	1	no
writing-quality	whole	default	claims-emitted	1	no
test-trimming	whole	default	no-claims	1	no
restated-sets	whole	default	claims-emitted	1	no

**Review** `01M41Q08APM5XD2ZASJB6K2CVY` — head `3be32e1aea064149a5fa858639ee38959c3b4e2d` # Review — j4k-oss/review-wrapper @ c032678d1064 Scope: diff against base tree `af4a01b58937` Status: dispatched — coverage complete (4/4 slots terminal) Facts: current review-wide projection Computed under: ```json { "abandonment": "abandonment-v1", "anchor_recipe": 1, "batch_policy": "batch-v1", "coverage": "coverage-v3", "dispatch_policy": "dispatch-v2", "grounder_version": 1, "grounding_read_rule": "grounding-read-v1", "promotion_policy": "promotion-v1", "report": "report-v3", "tally": "tally-v1", "triage_settle": "triage-settle-v2" } ``` ## Findings (1) ### low — Describe destination conversion accurately in the regex comment - claim: `01M41Q7ZAE16K8X30SYDA4RGV7` - anchor: `src/markdown/neutralize-html.ts` (snippet) ``` /** A backslash escape, or a character only an angle-bracket destination may hold. */ ``` - lens: writing-quality · arm: default - verdicts: 1 valid / 0 invalid / 0 uncertain - duplicates: `01M41QAFSAXXKTM8TW25VA1A0A` (restated-sets) - disposition: none > I read the DESTINATION_UNSAFE declaration, plainDestination, and the new neutralize-html test for a backslash before whitespace. The comment says the second regex branch contains characters only an angle-bracket destination may hold. That branch now includes a bare backslash; the converter also emits escaped parentheses in a plain destination. The distinction a maintainer needs is which characters require conversion to preserve the destination, so the present explanation can lead them to remove the backslash case that this change fixes. Replace the comment with: 'Backslash escapes to preserve and characters this converter encodes or escapes when removing destination brackets.' This preserves the regex's purpose without claiming those characters are exclusive to bracketed destinations. The mismatch follows from the pattern, conversion function, and regression assertion; I did not verify renderer output independently. ## Other claims - grounding-pending (0) - ungrounded (0) - rejected (0) - duplicate-of (1) - `01M41QAFSAXXKTM8TW25VA1A0A` high — DESTINATION_UNSAFE comment omits standalone backslashes from its match set → `01M41Q7ZAE16K8X30SYDA4RGV7` - unadjudicated (0) ## Coverage Coverage pass: 01M41Q08D84CMFX59F5HAQHJ93 Accounting: complete Slot health: healthy | lens | part | arm | unit status | runs | loss | | --- | --- | --- | --- | --- | --- | | general-bug | whole | default | no-claims | 1 | no | | writing-quality | whole | default | claims-emitted | 1 | no | | test-trimming | whole | default | no-claims | 1 | no | | restated-sets | whole | default | claims-emitted | 1 | no |

🚀 1

forgejo-actions reviewed

2026-10-03 20:31:52 +00:00

src/markdown/neutralize-html.ts

					
				@ -18,7 +18,7 @@ const TAG_START = /<(?:[/!?]|\p{L}[\p{L}\p{N}-]*(?=[\s/>]|$))/gu;

				const PARSE_MAX = 4096;

				/** A backslash escape, or a character only an angle-bracket destination may hold. */

low — Describe destination conversion accurately in the regex comment

I read the DESTINATION_UNSAFE declaration, plainDestination, and the new neutralize-html test for a backslash before whitespace. The comment says the second regex branch contains characters only an angle-bracket destination may hold. That branch now includes a bare backslash; the converter also emits escaped parentheses in a plain destination. The distinction a maintainer needs is which characters require conversion to preserve the destination, so the present explanation can lead them to remove the backslash case that this change fixes. Replace the comment with: 'Backslash escapes to preserve and characters this converter encodes or escapes when removing destination brackets.' This preserves the regex's purpose without claiming those characters are exclusive to bracketed destinations. The mismatch follows from the pattern, conversion function, and regression assertion; I did not verify renderer output independently.

lens writing-quality · arm default · tally 1 valid / 0 invalid / 0 uncertain
claim 01M41Q7ZAE16K8X30SYDA4RGV7 of review 01M41Q08APM5XD2ZASJB6K2CVY

**low** — Describe destination conversion accurately in the regex comment > I read the DESTINATION_UNSAFE declaration, plainDestination, and the new neutralize-html test for a backslash before whitespace. The comment says the second regex branch contains characters only an angle-bracket destination may hold. That branch now includes a bare backslash; the converter also emits escaped parentheses in a plain destination. The distinction a maintainer needs is which characters require conversion to preserve the destination, so the present explanation can lead them to remove the backslash case that this change fixes. Replace the comment with: 'Backslash escapes to preserve and characters this converter encodes or escapes when removing destination brackets.' This preserves the regex's purpose without claiming those characters are exclusive to bracketed destinations. The mismatch follows from the pattern, conversion function, and regression assertion; I did not verify renderer output independently. lens `writing-quality` · arm `default` · tally 1 valid / 0 invalid / 0 uncertain claim `01M41Q7ZAE16K8X30SYDA4RGV7` of review `01M41Q08APM5XD2ZASJB6K2CVY`

🚀 1

jercik marked this conversation as resolved

jercik commented

2026-10-03 20:33:39 +00:00

Replying to review comment #110702

The wording could describe conversion more precisely. This PR is authorized only to migrate the exact frozen canonical #101 regexp fix; its mirror must preserve the canonical source and comment. 3be32e1aea064149a5fa858639ee38959c3b4e2d matches the canonical committed helper beyond the ownership comment, and native Goldmark regressions verify the URL behavior through the actual callers. A wording change would require separate canonical-source and mirror work, which is outside this migration's authorization. Acknowledged as a comment clarification left unchanged, rather than a behavioral fix or a disagreement with the suggestion.

> Replying to review comment #110702 The wording could describe conversion more precisely. This PR is authorized only to migrate the exact frozen canonical #101 regexp fix; its mirror must preserve the canonical source and comment. `3be32e1aea064149a5fa858639ee38959c3b4e2d` matches the canonical committed helper beyond the ownership comment, and native Goldmark regressions verify the URL behavior through the actual callers. A wording change would require separate canonical-source and mirror work, which is outside this migration's authorization. Acknowledged as a comment clarification left unchanged, rather than a behavioral fix or a disagreement with the suggestion.

jercik referenced this pull request

2026-10-03 20:47:05 +00:00

docs: clarify destination conversion comment #27

jercik commented

2026-10-03 20:48:05 +00:00

Replying to review comment #110702

Tracked in #27. Commit 229c5803be8ce5f762f2a20c5024460134e6cf89 clarifies the destination-conversion comment in a separate PR targeting main; it changes no regexp or URL behavior. PR #26 remains at the frozen 3be32e1aea064149a5fa858639ee38959c3b4e2d and the canonical trees remain untouched.

This corrects my earlier outcome in comment #110722: preserving this migration's scope requires separate ownership, not an acknowledgment without a follow-up. The governing Forgejo family workflow at setup-atlas 4093578b83c094856d65add8111aa878bb2407b4 routes separable wording improvements into actual follow-up PRs and requires their URL on the source finding. PR #27 is open and owned through its own review; this source finding is acknowledged as tracked, not fixed by an unmerged child.

> Replying to review comment #110702 Tracked in https://code.j4k.dev/j4k-oss/review-wrapper/pulls/27. Commit `229c5803be8ce5f762f2a20c5024460134e6cf89` clarifies the destination-conversion comment in a separate PR targeting `main`; it changes no regexp or URL behavior. PR #26 remains at the frozen `3be32e1aea064149a5fa858639ee38959c3b4e2d` and the canonical trees remain untouched. This corrects my earlier outcome in comment #110722: preserving this migration's scope requires separate ownership, not an acknowledgment without a follow-up. The governing Forgejo family workflow at setup-atlas `4093578b83c094856d65add8111aa878bb2407b4` routes separable wording improvements into actual follow-up PRs and requires their URL on the source finding. PR #27 is open and owned through its own review; this source finding is acknowledged as tracked, not fixed by an unmerged child.

jercik merged commit fbac849ecc into main

2026-10-03 21:13:07 +00:00

jercik referenced this pull request from a commit

2026-10-03 21:13:08 +00:00

fix: sync literal backslash URL preservation (#26)

Sign in to join this conversation.

No reviewers

No labels

No milestone

No assignees

2 participants

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

j4k-oss/review-wrapper!26

Rows
Columns