Show release notes in the update popup, sourced from CHANGELOG.md - #7432
Conversation
The update banner only linked out to the online changelog, so there was no way to see what an update contains before taking it. Add CHANGELOG.md at the repo root as the source of release notes. Studio reads it from the default branch, so editing the file updates the popup without a release or rebuild, and falls back to the copy bundled in the install when the repo is unreachable. Notes are matched to one exact version. The popup asks for the version it is offering and gets that section or nothing, so an older release's notes can never appear next to a newer update. When there is no match the popup links out to the online changelog instead. The collapsed popup previews the top bullets with the leading sentence highlighted; "Show release notes" expands the full notes in a scrollable panel. Applies to both the browser and desktop banners, and the desktop updater's own release body is used when CHANGELOG.md has no matching section.
for more information, see https://pre-commit.ci
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1960fd179d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if _FENCE_PATTERN.match(line): | ||
| in_fence = not in_fence |
There was a problem hiding this comment.
When a changelog uses a longer outer fence to demonstrate fenced Markdown (for example, four backticks containing a triple-backtick block), toggling on every line that starts with either fence marker closes the outer block at the inner delimiter. A sample ## <version> inside that block is then indexed as a real release and can supply incorrect notes or terminate the actual section. Track the opening marker and length, and only close on a compatible delimiter.
Useful? React with 👍 / 👎.
| continue; | ||
| } | ||
| if (!inFence) { | ||
| lines.push(rawLine.replace(BLOCKQUOTE, "").trim()); |
There was a problem hiding this comment.
When release notes contain nested lists, trimming every line before applying BULLET erases the indentation that distinguishes child items from top-level changes. The preview consequently counts nested details as separate headline bullets, so they can consume the four-item limit and hide later top-level changes even though this function explicitly promises a top-level-bullet preview. Preserve list indentation through parsing and exclude nested items from the preview.
Useful? React with 👍 / 👎.
| open={notesOpen} | ||
| // Updater's release body, used only if CHANGELOG.md has no | ||
| // section for this version. | ||
| fallbackMarkdown={info?.body ?? null} |
There was a problem hiding this comment.
For Linux installs using manual_linux_package, this fallback is always empty: .github/workflows/release-desktop.yml writes the release text to the updater metadata as notes (lines 911-915), while ChannelMetadata in studio/src-tauri/src/desktop_update_policy.rs deserializes only body and then forwards that missing field (lines 34-38 and 99-103). Therefore, whenever CHANGELOG.md has no matching desktop-version section—the exact case this fallback is meant to cover—the manual Linux update popup cannot display the published release body. Map the updater's notes field into info.body before passing it here.
Useful? React with 👍 / 👎.
|
Thanks, went through all of these. Three were right and are fixed in a6f8dc6, plus one more I found while testing. Fence matching (P2) - correct. A ``` sample inside a ```` block closed the outer block, and the sample's Nested list bullets (P2) - correct, and it contradicted the docstring. Nested items were counted as headline changes and ate the four-item limit. Indentation is now preserved through parsing and only top-level bullets are previewed, with the shallowest bullet defining top level so a uniformly indented list still works. Old logic gave Manual Linux updater notes (P2) - correct, and the trace was right. CodeQL incomplete multi-character sanitization - fixed, but worth being precise about what it was. The value is rendered as a React text node and never as HTML, so there was no injection path. I also searched exhaustively over every string up to length 8 of Found while testing: UTF-8 BOM. A BOM on the first line hid a section whose heading started the file, which is easy to introduce from an editor on Windows. Stripped before parsing. Testing behind this, all outside the repo in a scratch venv: 95 backend cases (parsing, version matching, remote/local sourcing, HTTP failures, cache single-flight under 24 threads, API contract, platform edge cases), 51 preview cases, 6 Rust metadata cases, and 51 cross-engine layout checks driving the real component in Chromium, Firefox and WebKit at desktop and mobile widths. The shipped suite is now 27 tests and covers the fence, BOM, CRLF and updater-field regressions. The |
Track the opening fence marker and length so a ``` sample inside a ```` block does not close it early and let the sample's heading be indexed as a real release. Preserve list indentation in the preview and take only top-level bullets, so nested detail no longer consumes the four headline slots. Strip a UTF-8 BOM before parsing. An editor on Windows can leave one on the first line, which hid a section whose heading started the file. Read `notes`/`pub_date` from latest.json in the manual Linux updater path, with aliases for the older `body`/`date`. The workflow publishes Tauri's field names, so the manual path's release body was always empty. Also loop the preview tag strip until stable for CodeQL js/incomplete-multi-character -sanitization; the value renders as text, so this is defence in depth.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 299d36799e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| RELEASE_NOTES_MAX_CHARS = 20_000 | ||
|
|
||
| _HEADING_PATTERN = re.compile(r"^##\s+(?P<title>.*?)\s*$") | ||
| _FENCE_PATTERN = re.compile(r"^\s*(?P<marker>`{3,}|~{3,})") |
There was a problem hiding this comment.
Fresh evidence after the delimiter-length fix is that _FENCE_PATTERN still accepts trailing non-whitespace: inside a four-backtick block, a content line such as ```` not a closeris treated as the closing fence even though Markdown requires a closing fence to contain only the delimiter and optional whitespace. A following sample## 9.9.9` is consequently indexed as a real release and can return incorrect notes; distinguish opening fences with info strings from closing fences and require valid closing-line syntax.
Useful? React with 👍 / 👎.
| } | ||
| }, [open, markdown]); | ||
|
|
||
| const notesUrl = notes?.releaseNotesUrl ?? releaseNotesUrl; |
There was a problem hiding this comment.
When a desktop update has no matching CHANGELOG.md section, markdown correctly selects the updater-provided fallbackMarkdown, but the successful unmatched API response still contains the generic backend changelog URL, so this nullish expression always ignores manualReleaseUrl. This is the normal desktop fallback case because desktop releases use separate SemVer versions; the displayed GitHub release body therefore links users to an unrelated generic changelog instead of the exact desktop release page passed by the banner.
Useful? React with 👍 / 👎.
| for parent in here.parents[1:5]: | ||
| candidates.append(parent / CHANGELOG_FILENAME) |
There was a problem hiding this comment.
After build.sh creates the ignored studio/CHANGELOG.md, this parent-order checks that snapshot before the repository-root changelog, and _read_local_changelog returns the first existing file without trying later candidates. Consequently, editing the documented canonical root CHANGELOG.md after any local build still serves the stale snapshot whenever remote fetching is disabled or unavailable—even when only the root file contains the requested version. In source checkouts, prioritize the repository root or remove the generated snapshot after packaging.
Useful? React with 👍 / 👎.
| /** Inline markdown stripped to plain text. */ | ||
| function toPlainText(markdown: string): string { | ||
| return stripHtmlTags(markdown.replace(IMAGE, "").replace(LINK, "$1")) | ||
| .replace(EMPHASIS, "") |
There was a problem hiding this comment.
Whenever a release-note bullet names an environment variable or identifier, the unconditional emphasis replacement removes every underscore rather than only paired Markdown delimiters. For example, UNSLOTH_DISABLE_UPDATE_CHECK is shown in the collapsed preview as UNSLOTHDISABLEUPDATECHECK, turning an exact configuration name into an incorrect one; strip paired emphasis syntax while preserving literal identifier characters.
Useful? React with 👍 / 👎.
| # A `##` inside a fenced block is sample markdown, not a real heading. | ||
| match = None if open_fence else _HEADING_PATTERN.match(line) |
There was a problem hiding this comment.
When maintainers comment out a prospective release section with <!-- ... -->, Markdown does not render the enclosed ## <version> as a heading, but this parser still matches it because it tracks fenced blocks only. The commented section is therefore indexed and can expose unpublished notes for an offered version, or prematurely terminate the preceding release body; track multiline HTML comments before recognizing headings.
Useful? React with 👍 / 👎.
… URL A closing fence must carry nothing after the delimiter, so a ```` line with trailing text inside a ```` block is content rather than the end of it. Both the parser and the preview extractor follow that rule now. Skip headings inside HTML comments. A commented-out section is not rendered by Markdown, so it must not be indexed as a release. Strip only paired emphasis and park code spans first, so identifiers keep their underscores: UNSLOTH_DISABLE_UPDATE_CHECK was previewing as UNSLOTHDISABLEUPDATECHECK. Prefer the caller's release URL over the API's generic changelog link, so the desktop fallback points at the release page for the version being offered. Look at the repo-root CHANGELOG.md before the packaging snapshot, and remove the snapshot after build.sh, so an edited root file is never shadowed by a stale copy. Also nudge the notes container radius from 16px to 14px.
|
Second round, all five checked against the code first. Every one reproduced, fixed in 0ed5974. Bare fence closers - correct, and my first fix was incomplete. Length matching alone was not enough: a Headings in HTML comments - correct. Identifier underscores - correct and the most user-visible of the five. Fallback release URL - correct. The API always returns the generic changelog URL, so Root changelog vs build snapshot - correct. Verification: 33 shipped tests, 104 backend simulation cases, 65 preview cases including guards against the code-span placeholder colliding with plain numbers, and 51 cross-engine layout checks in Chromium, Firefox and WebKit. The old logic was replayed against each input to confirm the fix changes the outcome rather than only the code. Also nudged the notes container radius from 16px to 14px, confirmed rendering at 14px in all three engines. |
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e304d21e3d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const BLOCKQUOTE = /^\s*>\s?/; | ||
| const IMAGE = /!\[[^\]]*\]\([^)]*\)/g; | ||
| const LINK = /\[([^\]]*)\]\([^)]*\)/g; | ||
| const HTML_TAG = /<[^>]*>/g; |
There was a problem hiding this comment.
When a release-note bullet contains a visible constraint such as Support Python <3.15 and >3.9, this pattern removes <3.15 and > as though it were an HTML tag, so the collapsed popup misleadingly displays Support Python 3.9. Match only syntactically valid HTML tags, or rely on the text-only renderer, so ordinary comparison operators remain intact.
Useful? React with 👍 / 👎.
| const stripped = line.replace(BLOCKQUOTE, ""); | ||
| lines.push({ | ||
| text: stripped.trim(), | ||
| indent: stripped.length - stripped.trimStart().length, | ||
| }); |
There was a problem hiding this comment.
When a release section contains a multiline comment such as <!-- followed by - unpublished change, these lines are collected like ordinary content and the hidden bullet appears in the collapsed preview, even though the expanded Markdown view does not render it. Track HTML-comment state while producing content lines so commented-out release notes cannot be exposed as published changes.
Useful? React with 👍 / 👎.
| error = remote.error if (remote.error and not local.text) else None | ||
| return _notes_response(version = version, error = error) |
There was a problem hiding this comment.
When the bundled changelog is readable but lacks the offered newer version—the normal installed-package case—a remote fetch failure is discarded solely because local.text exists. The endpoint consequently returns an unmatched response indistinguishable from a confirmed missing release, and the panel says that no notes were published instead of entering its retryable error state; preserve the remote failure whenever neither source matched and ensure it reaches the hook as an error.
Useful? React with 👍 / 👎.
Require a name character after "<" when stripping tags. A bullet reading "Support Python <3.15 and >3.9" previewed as "Support Python 3.9", because the operators were consumed as if they were a tag. Track HTML comments while collecting preview lines. A commented-out bullet was previewed as a published change even though Markdown never renders it. Report a remote lookup failure whenever nothing matched. The bundled changelog cannot know a version newer than the install, so discarding the error made an offline lookup read as "no notes were published". The hook now treats a reported failure as its retryable error state.
|
Third round. Three new ones, all reproduced before I touched anything, fixed in b953304 (rebased onto pre-commit as 73fbc49). The other five in that batch were the round-two items, already fixed in 0ed5974. Comparison operators in preview text - correct, and the worst of the three. Multiline HTML comments in the preview - correct. I had added comment tracking to the parser last round but not to the preview extractor, so Swallowed remote failures - correct, and it was the common case rather than an edge one. With a bundled changelog present but no section for the offered version, Verification: 37 shipped tests, 107 backend simulation cases, 73 preview cases, 51 cross-engine layout checks in Chromium, Firefox and WebKit. On CI: the only red check so far is |
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 786b5a90e0
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| index = close + len(_COMMENT_CLOSE) | ||
| in_comment = False | ||
| continue | ||
| opening = line.find(_COMMENT_OPEN, index) |
There was a problem hiding this comment.
When a release note documents an HTML comment delimiter in inline code, such as - Type `<!--` to begin a comment, this search treats the literal code span as a real comment opener and remains in comment state across following lines. Subsequent version headings are then swallowed into the current entry, so the earlier version can display another release's notes while the later version cannot be found; comment tracking needs to exclude delimiters protected by Markdown code spans.
Useful? React with 👍 / 👎.
| const { state, notes, retry } = useReleaseNotes({ version, enabled: true }); | ||
| const scrollRef = useRef<HTMLElement | null>(null); | ||
|
|
||
| const markdown = notes?.matched ? notes.markdown : (fallbackMarkdown ?? null); |
There was a problem hiding this comment.
If the offered version changes while this component remains mounted, notes still contains the previous version during the first render because the hook only clears it later in an effect. This expression consequently renders the old release's summary next to the new version for a render, despite the exact-version guarantee; require notes.version === version here or otherwise invalidate the state synchronously.
Useful? React with 👍 / 👎.
| const retry = useCallback(() => { | ||
| if (version) { | ||
| requestedVersionRef.current = null; | ||
| load(version); |
There was a problem hiding this comment.
When the first remote lookup fails and connectivity then recovers, this retry merely repeats the same request, while get_remote_changelog() returns its cached failure for CHANGELOG_FAILURE_TTL_SECONDS (five minutes) without attempting the network. The visible Retry action therefore cannot recover until the cache expires; thread an explicit refresh through the endpoint or otherwise invalidate cached failures when the user retries.
Useful? React with 👍 / 👎.
…failures Treat an HTML comment delimiter inside inline code as literal. A note reading "Type `<!--` to begin a comment" put the parser into comment state, so every release below it was swallowed into the entry above and became unfindable. Applied to the preview extractor too. Return no notes while the offered version differs from the fetched one. On the render where the version changes, the hook still held the previous release's notes, which the panel would show for a frame. Let retry bypass a cached remote failure via a refresh flag on the endpoint. Failures are cached for five minutes, so the visible Retry action could not recover until the TTL expired. A cached success is still reused, so retries cannot hammer the remote.
|
Fourth round. All three reproduced first, fixed in 1729ba6. Comment delimiters inside inline code - correct, and the most damaging one raised so far. A note reading Stale notes for a render - correct. The hook cleared state in an effect, so on the render where the offered version changed, the previous release's notes were still in state and the collapsed preview could paint them next to the new version. The hook now returns nothing unless the fetched notes are for the requested version, so every consumer is covered rather than just the panel. Retry stuck behind the cached failure - correct. Failures cache for five minutes, so Retry replayed the cached failure and could not recover once connectivity returned. Added a refresh flag on the endpoint that drops a cached failure only; a cached success is still reused so retries cannot be used to hammer the remote. Verified by hit-counting a failing server: two calls, one request; then refresh, two requests. Verification: 40 shipped tests, 114 backend simulation cases, 76 preview cases, 51 cross-engine layout checks in Chromium, Firefox and WebKit. One note on a test I wrote and then corrected: I initially asserted that a section body should exclude commented-out content. That is wrong. Bodies keep raw Markdown because the renderer hides comments, and the collapsed preview strips them separately, so the assertion now checks that behaviour instead. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1729ba6c14
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Updater's release body, used only if CHANGELOG.md has no | ||
| // section for this version. | ||
| fallbackMarkdown={info?.body ?? null} | ||
| releaseNotesUrl={manualReleaseUrl} |
There was a problem hiding this comment.
For macOS, Windows, and AppImage updates, manualReleaseUrl is always null because useTauriUpdate constructs it only in manual_linux_package mode. Since desktop SemVer releases normally use info.body as the fallback, this makes the panel's “Open changelog” link resolve to the generic backend changelog instead of the corresponding desktop-v<version> release. Fresh evidence beyond the earlier URL-precedence fix is the mode gate at use-tauri-update.ts:387-390; provide an exact release URL for in-app desktop updates too.
Useful? React with 👍 / 👎.
| RELEASE_NOTES_MAX_CHARS = 20_000 | ||
|
|
||
| _HEADING_PATTERN = re.compile(r"^##\s+(?P<title>.*?)\s*$") | ||
| _FENCE_PATTERN = re.compile(r"^\s*(?P<marker>`{3,}|~{3,})(?P<rest>.*)$") |
There was a problem hiding this comment.
When a changelog contains a four-space-indented line beginning with backticks or tildes, Markdown treats it as indented code rather than a fenced-block opener, but \s* accepts it here and sets open_fence. If no matching delimiter follows, subsequent real ## <version> headings are swallowed and their notes are appended to the preceding release, violating the exact-version guarantee. Accept at most three leading spaces when recognizing fences.
Useful? React with 👍 / 👎.
| CHANGELOG_FAILURE_TTL_SECONDS = 5 * 60 | ||
| RELEASE_NOTES_MAX_CHARS = 20_000 | ||
|
|
||
| _HEADING_PATTERN = re.compile(r"^##\s+(?P<title>.*?)\s*$") |
There was a problem hiding this comment.
A Markdown ATX heading may have up to three leading spaces, so a changelog section such as ## 2026.8.0 is rendered as a real level-two heading by the expanded Markdown view. This anchored pattern only recognizes headings beginning in column zero, causing that version to be unavailable and potentially leaving its content attached to the preceding release. Permit the standard zero-to-three-space indentation before ##.
Useful? React with 👍 / 👎.
Allow up to three leading spaces on release headings and fences, and treat four as indented code. An indented heading was unreachable and its notes were appended to the release above, while an indented backtick line opened a fence that swallowed later headings. Link desktop release notes to the release page for the offered version on every platform. The existing URL is built only in manual Linux package mode, so in-app updates on macOS, Windows and AppImage fell back to the generic changelog. The install button keeps using the manual URL.
|
Fifth round. All three checked against the code first and all three held up, fixed in 42ccaf8. Heading indentation - correct. Markdown renders Fence indentation - correct and the mirror image. A four-space indented backtick line is indented code, not a fence opener, but Desktop release URL - correct, and the gate is exactly where you pointed. Verification: 46 shipped tests, 124 backend simulation cases, 78 preview cases, 51 cross-engine layout checks. The indentation cases are parameterised across zero to three spaces, with four asserted to behave as code in both directions. CI: no failures introduced. The one red check remains |
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8e70d0c59d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if open_fence is None: | ||
| return marker |
There was a problem hiding this comment.
When a changelog line starts with a backtick run but its info string also contains a backtick (for example, three backticks followed by `literal`), CommonMark does not treat it as a fenced-code opener. This branch nevertheless sets open_fence, causing subsequent real release headings to be suppressed until another compatible delimiter or EOF, so their notes become unavailable or remain attached to the preceding release. Reject backtick fence openers whose info strings contain backticks, and keep the mirrored frontend preview parser aligned.
Useful? React with 👍 / 👎.
| # Remote first: the offered version is newer than the local copy. | ||
| for candidate in (remote, local): | ||
| if not candidate.text: | ||
| continue | ||
| entry = find_release_notes(candidate.text, version) |
There was a problem hiding this comment.
When the remote changelog is fetched successfully but no longer contains the requested version while the bundled changelog still does, this loop falls through and returns the bundled entry. Online clients can therefore display stale or retracted notes even though the default-branch copy is described as authoritative and the bundled copy is intended only as an offline fallback; consult the local copy only when the remote source is unavailable or disabled.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Below the bar as described. The behaviour is real, but the update popup only ever asks for a version newer than the one installed, which a build-time bundled snapshot cannot contain, so the stale-notes path is not reachable in practice. The response also reports source so a caller can distinguish local from remote.
| const text = toPlainText(line.text); | ||
| if (current === null) { | ||
| if (text) { | ||
| prose.push(truncate(text)); |
There was a problem hiding this comment.
When release notes use an ordinary paragraph instead of bullets and that paragraph is wrapped across source lines, every nonblank line is appended to prose independently. Markdown renders those lines as one paragraph, but the collapsed preview displays them as separate bullet-like fragments that can consume the four-item limit; accumulate contiguous prose lines and flush them only at paragraph or heading boundaries.
Useful? React with 👍 / 👎.
| const LINK = /\[([^\]]*)\]\([^)]*\)/g; | ||
| // Real tags only: a name character must follow "<", so a version constraint | ||
| // like "Support Python <3.15 and >3.9" keeps its operators. | ||
| const HTML_TAG = /<\/?[a-zA-Z][^>]*>/g; |
There was a problem hiding this comment.
For a standard Markdown autolink such as <https://example.com/notes>, this expression treats the entire URL as an HTML tag because the first character after < is a letter. stripHtmlTags consequently removes the visible link text from the collapsed preview even though the expanded Markdown renders it; convert autolinks to plain text before stripping tags or restrict this matcher to actual tag syntax.
Useful? React with 👍 / 👎.
| const PARKED = /\uE000(\d+)\uE001/g; | ||
| const WHITESPACE = /\s+/g; | ||
| // Sentence end followed by something that actually starts a sentence. | ||
| const SENTENCE_BREAK = /[.!?]\s+(?=["'“‘]?[A-Z0-9])/; |
There was a problem hiding this comment.
When a bullet contains a common abbreviation followed by an uppercase continuation, such as Supports several formats, e.g. GGUF and Safetensors., this pattern treats the final period in e.g. as a sentence boundary. The preview highlights only the setup through e.g. and dims the actual change, contrary to the leading-sentence behavior; exclude common abbreviation forms before selecting the split.
Useful? React with 👍 / 👎.
Accumulate contiguous prose lines into one preview item. A paragraph wrapped across source lines renders as one block but previewed as three fragments, which also ate the four-item limit. Keep Markdown autolinks. <https://example.com/notes> was stripped as if it were a tag, so "See <https://example.com/notes> for details" previewed as "See for details". Do not split the lead sentence at an abbreviation. "Supports several formats, e.g. GGUF and Safetensors." highlighted only up to "e.g." and dimmed the actual change; known abbreviations and single initials are skipped now.
|
Sixth round. All three reproduced against the real extractor before I changed anything, fixed in 26c5d2e. Wrapped prose - correct. A paragraph wrapped across three source lines previewed as three separate items and ate three of the four slots, even though Markdown renders it as one block. Contiguous prose now accumulates and flushes at a paragraph or heading boundary, so it previews as one item and separate paragraphs stay separate. Autolinks - correct. Abbreviations - correct. "Supports several formats, e.g. GGUF and Safetensors." split at Verification: 47 shipped tests, 124 backend simulation cases, 88 preview cases, 51 cross-engine layout checks in Chromium, Firefox and WebKit. CI: nothing to fix from this branch. Every red check so far has been |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 26c5d2e951
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const parked = stripHtmlTags( | ||
| markdown.replace(AUTOLINK, "$1").replace(IMAGE, "").replace(LINK, "$1"), | ||
| ).replace(CODE_SPAN, (_match, code: string) => { |
There was a problem hiding this comment.
When a release-note bullet contains literal Markdown or HTML inside a code span, the replacements run before the span is parked. For example, Use `<button>` for actions loses <button> to stripHtmlTags, and a literal link such as `[label](url)` is rewritten before code-span handling, so the collapsed preview differs from the expanded notes. Extract and park code spans before applying autolink, image, link, HTML, and emphasis transformations.
Useful? React with 👍 / 👎.
| const bullet = BULLET.exec(line.text); | ||
| if (bullet) { | ||
| flush(); | ||
| current = { text: toPlainText(bullet[1] ?? ""), indent: line.indent }; |
There was a problem hiding this comment.
When a release section contains a four-space-indented code block with a line beginning in a list marker, such as an indented - pip install ... command, contentLines preserves the indent but trims the text and this branch classifies it as a bullet. If the section has no real list, that code line becomes the collapsed headline and causes accompanying prose to be ignored, even though the expanded Markdown renders it as code. Track indented code blocks and exclude their lines before applying BULLET.
Useful? React with 👍 / 👎.
Park code spans before any other inline transformation. Tags, links, images and emphasis inside a span are literal, but the strips ran first, so "Use `<button>` for actions" previewed as "Use for actions". Skip lines inside an indented code block when collecting bullets. A "- pip install ..." line in a four-space-indented block became the headline and pushed out the real prose, though Markdown renders it as code. Continuation lines of an open bullet are unaffected.
|
Seventh round. Both reproduced, fixed in aca4be1. Code spans parked too late - correct. The autolink, image, link and tag strips all ran before parking, so Indented code read as a bullet - correct. With a four-space-indented block containing Verification: 48 shipped tests, 124 backend simulation cases, 100 preview cases, 51 cross-engine layout checks in Chromium, Firefox and WebKit. One correction on my side: I first asserted that CI: still no failure attributable to this branch. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aca4be108c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # Commented-out sections are not rendered, so they are not releases. | ||
| visible, in_comment = _strip_comments(line, in_comment) | ||
| # A `##` inside a fenced block is sample markdown, not a real heading. | ||
| match = _HEADING_PATTERN.match(visible) if visible else None |
There was a problem hiding this comment.
When a release uses a raw HTML block such as <pre> or <script> to display Markdown examples, CommonMark treats the block contents literally, but this parser suppresses only fenced code and comments. A line like ## 9.9.9 inside <pre> therefore flushes the real release and creates a fake version entry, causing the real notes to be truncated and the sample version to become queryable; track raw HTML block state before matching headings.
Useful? React with 👍 / 👎.
| const bullet = BULLET.exec(line.text); | ||
| if (bullet) { | ||
| flush(); | ||
| current = { text: toPlainText(bullet[1] ?? ""), indent: line.indent }; |
There was a problem hiding this comment.
When release notes contain raw code HTML such as <pre> followed by - pip install ..., contentLines passes the literal code line here and it is classified as a release bullet. The collapsed preview can consequently promote a command from the code sample and hide later real changes, even though the expanded Markdown renders that line as code; skip content inside raw <pre>/script/style blocks before applying the bullet matcher.
Useful? React with 👍 / 👎.
A <pre>, <script>, <style> or <textarea> block renders literally, so a sample '## 9.9.9' heading inside one was indexed as a release and cut the real section's body short. The preview had the same gap and listed sample bullets as notes. Both readers now track type 1 HTML blocks and skip their contents. Blocks open only at the start of a line, so a tag named mid-sentence stays inline text, and <details> is type 6 so its Markdown still parses.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ba9150236f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # Commented-out sections are not rendered, so they are not releases. | ||
| visible, in_comment = _strip_comments(line, in_comment) |
There was a problem hiding this comment.
When a release heading with the supported 1–3 leading spaces follows an unindented HTML comment after a list (for example, - old item\n<!-- separator -->\n ## 2.0), _strip_comments blanks the comment before list tracking sees it. The old list therefore remains open, the heading is treated as nested list content, and the parser merges the new release into the preceding section instead of indexing its notes. Keep the comment opener's structural indentation available to _open_lists even though it is hidden from heading matching.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Correct. The oracle closes the list at a margin-level comment and emits <h2>2.0</h2> at document level; the scanner returned one release holding the second one's heading and notes.
_strip_comments blanks the line before list tracking sees it, so the fix gives the hidden line its own structural stand-in: leading whitespace plus a #, which carries the real column while offering no list marker and no lazy continuation. Extending it to continuation lines as well regressed two shapes, so it is limited to the line that opens the block. Ported to markdown-list-columns.ts and consumed by both frontend scanners, where it fixes two visible cases of its own: the resolver no longer rewrites a link inside an indented code block, and a preview no longer drops a real second bullet behind a phantom list-scoped fence.
Fuzzing the comment corpus against the oracle: 156 mismatches to 63, with five seeds each fixing 71 to 93 documents and regressing none. Port parity re-checked at 26,861 steps with no mismatches. Fixed in b72fcf4.
| const INLINE_TARGET = new RegExp( | ||
| String.raw`(!?)\[${NESTED_LABEL}\]\(\s*(<[^<>\n]*>|(?:\\.|[^\s()])*)`, | ||
| "g", |
There was a problem hiding this comment.
When a valid inline link destination begins with a balanced parenthesis, such as [details]((draft).md), CommonMark renders it as a link to (draft).md, but this expression matches an empty destination because ( is excluded from its first character. absolute() therefore leaves the Markdown unchanged, and clicking the link resolves it against Studio's own origin rather than the Unsloth repository. Parse balanced destination parentheses instead of stopping before an initial (.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Correct. [details]((draft).md) links to (draft).md and .png) sources (a).png; both were left untouched and resolved against Studio's origin. The preview's own destination expression already allowed one level of nesting, so the two frontend scanners disagreed with each other.
Worth recording that the naive fix is wrong: allowing balanced pairs outright regressed 609 documents, because [x](a(b.md) is not a link in CommonMark and the greedy group swallowed the closer and invented one that ended on a later line. The balanced alternative is now gated on the closer being followed by whitespace, a quote or ), with the paren-free expression as the fallback, so an unbalanced paren stays the closer.
Fuzzed against the oracle on hrefs and on verbatim code and HTML text: paren-heavy 3,803 fixed and 0 regressed over 8,000 documents, and 0 regressions on the two existing corpora. The pathological-input guard is unmoved at 978 ms against 977 ms, and five new paren-pathological inputs all stay under 37 ms. Fixed in b72fcf4.
| # A dashed underline is not a list marker, so track lists after setext. | ||
| lists = _open_lists(structural, lists, after_paragraph) |
There was a problem hiding this comment.
When an indented code block is followed by an ordered marker starting at a number other than 1 (for example, code\n\n2. explanatory text), the Markdown renderer treats that marker as an ordinary paragraph, but after_paragraph is false here and _open_lists opens a list at content column 3. A subsequent valid three-space-indented release heading is then suppressed as nested content, so its notes are merged into the prior release. Preserve the indented-code transition state so this non-interrupting marker does not create a list container.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This one does not hold. The rule that an ordered list may interrupt only when it starts at 1 applies to paragraphs, and an indented code block is not one, so the list does open here and the three-space heading really is nested inside it. The oracle renders ## 1.0, a four-space code line, 2. explanatory text, then a three-space ## 2.0 as <pre><code> followed by <ol start="2"><li><p>explanatory text</p><h2>2.0</h2></li></ol>, so suppressing that heading is the correct reading, after_paragraph being false after indented code is correct, and opening at content column 3 is what the renderer does.
I could not build a repro. The scanner matches the oracle on six probe shapes and on an exhaustive 300-case product over three code shapes, five ordered markers, a gap, five heading indents and a tail, with no mismatches. A wider ordered-marker fuzz does show mismatches, but they are unrelated shapes, empty ordered items and lazy blockquote continuation, and are byte-identical before and after this PR. No change made.
A comment is an HTML block, so one written at the margin under a bullet is not indented enough to continue that item and closes the list. All three scanners blanked the line before list tracking saw it, which reads as a blank line and leaves the item open, so a release heading below it looked like nested item content and the new release merged into the one above. A hidden line now keeps its own column through _hidden_structure and hiddenStructure, and only its column, since the text a comment or a raw block hides is not Markdown and must not open a list of its own. A line inside a block already open is that block's content and still keeps nothing. A link destination may hold parentheses while they balance, so [x]((draft).md) points at (draft).md. The resolver stopped at the first paren, matched an empty destination and left the markdown alone, so the link resolved against Studio's own origin. The balanced form counts only while a closing paren or a title still ends the link, so the stray paren in [x](a(b.md) stays the closer the way CommonMark reads it rather than being swallowed into a link across lines.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b72fcf440c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| // A comment cannot open a fence and a fence hides a comment opener, so | ||
| // resolve them in that order or a hidden delimiter opens a phantom fence. | ||
| const fenceSource = inComment ? null : FENCE.exec(original); |
There was a problem hiding this comment.
When a fenced code block is the first content of a list item, such as - ```md followed by [example](docs/a.md), CommonMark treats the following lines as literal code. Testing FENCE against the unstripped line misses that opener, so the resolver rewrites the code sample's URL and the parallel preview scanner presents md example as a release-note bullet. Strip the list marker before detecting the fence so its contents remain literal in both views.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Correct, and not already covered by the container work from the previous commit: that strips the columns of items already open, but the tracker state is the line above, so on - ```md the marker is still on the line and the fence is missed. All three scanners had it, and the preview turned the code sample into a bullet reading md example.
A shared itemContent now strips the marker before block detection, with the padding capped exactly the way the list tracker caps it so - ``` stays indented code, and skipped when the marker is lazy text of the paragraph above. It had to cover HTML-block openers too, since scoping those without it produced seven new backend mismatches rooted in - <div>. Fixed in 02ad04c.
| if (close === -1) { | ||
| // Nothing closes it on this line, so the renderer shows it as text. | ||
| visible += line.slice(index); | ||
| break; |
There was a problem hiding this comment.
Unlike an unmatched opener followed by a new block, an inline opener whose closing --> appears on a lazy paragraph-continuation line forms a valid multiline HTML comment; for example, Visible <!-- hidden followed by secret --> end renders without hidden secret. This branch instead preserves the opener and resets comment state, causing the collapsed preview to expose the hidden text (and the link resolver has the same classification mismatch). Continue comment state only when a later line can validly complete the same paragraph.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Correct, and the distinction is exactly the paragraph's extent. The oracle gives one raw-HTML comment node for Visible <!-- hidden followed by secret --> end, and escapes the opener as literal text when a blank line or a heading intervenes, so the earlier round's rule was right for that shape and wrong for this one.
A new module decides, per line, whether the closer is reachable before the paragraph breaks, and lines carried by the comment keep their block structure and lose only their text. It is deliberately generous, so a wrong guess falls back to the previous behaviour rather than hiding content. The resolver had the same mismatch and is fixed with it. The backend needs nothing, since a heading always closes the paragraph, so no heading can sit inside one of these. Comment corpus: 1,265 render mismatches down to 671, with none new. Fixed in 02ad04c.
| elif in_html_block: | ||
| # A blank line is the only thing that ends a type 6 block. | ||
| in_html_block = line.strip() != "" |
There was a problem hiding this comment.
When a list item opens a type-6 HTML block on a continuation line, such as - item, a blank line, <div>, and then a dedented ## 2.0 without an intervening blank, CommonMark closes the HTML block when the list item ends and renders 2.0 as a document heading. This state only closes on a blank line, so it masks the dedented heading and every later line, making that release unqueryable; record the containing list column and clear the HTML-block state when the item ends.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Correct. ## 1.0, - item, blank, <div>, then ## 2.0 indexed only 1.0 while the oracle puts the second heading at document level, so that release and everything after it was unqueryable.
The block column is now recorded and cleared when the item ends, which is the analogue of the fence scoping already there. The link resolver already had it; the preview did not, and dropped a real bullet after a <div> opened inside an item, so both are fixed. Backend heading corpora went from 117, 88 and 17 mismatches to 16, 10 and 12, with none new. Fixed in 02ad04c.
|
|
||
| const NEEDS_BRACKETS = /[()\s]/; | ||
| // `\(` in a destination is a literal paren, not part of the path. | ||
| const ESCAPE = /\\(.)/g; |
There was a problem hiding this comment.
When a relative destination contains a backslash before a non-punctuation character, such as [guide](docs\alpha.md), CommonMark preserves that backslash because only ASCII punctuation is escapable. This expression removes every backslash regardless of the following character, rewriting the target as docsalpha.md and sending the user to a different repository path; restrict unescaping to CommonMark's escapable punctuation set.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Correct. [guide](docs\alpha.md) resolved to docsalpha.md on head; the oracle gives docs%5Calpha.md. Doubling the backslash would have been worse, since new URL reads it as a separator and yields docs/alpha.md, a different path again.
Unescaping is now restricted to CommonMark's escapable punctuation, in both the escape class and the destination character class, and a literal backslash is percent-encoded before URL resolution. One consequence worth noting: punctuation-only escaping means a space genuinely ends a destination, so [x](a b.md) and [x](a(b.md) are no longer half-rewritten, which also settles the unfiled case flagged last round. Multi-line links with their title on the next line still resolve. The markers and escapes corpus went from 8,823 destination mismatches to 104 with none new. Fixed in 02ad04c.
| _NOT_PARAGRAPH = re.compile( | ||
| r"^ {0,3}(?:#{1,6}([ \t]|$)" | ||
| r"|(?:\*[ \t]*){3,}$|(?:-[ \t]*){3,}$|(?:_[ \t]*){3,}$" | ||
| r"|\[(?:[^\[\]\\]|\\.)+\]:)" |
There was a problem hiding this comment.
When an open list paragraph is followed without a blank by reference-shaped text such as [ref]: x, CommonMark cannot parse it as a definition because definitions do not interrupt paragraphs, so it remains a lazy continuation of that item. This unconditional block classification closes the tracked list instead; a following two-space-indented ## 2.0 is then indexed as a document release even though the renderer keeps it nested in the list, truncating the real section and exposing unrelated notes. Treat a definition-shaped line as non-paragraph structure only when no paragraph is already open.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This one does not hold on head. The lazy admission already covers a definition-shaped line, and the block classification is already conditioned on no paragraph being open, so ## 1.0, - text, [ref]: x, ## 2.0 indexes only 1.0, which is the renderer's answer.
Checked rather than argued: an exhaustive search against markdown-it over roughly 30,000 definition-containing documents found zero mismatches of this shape at head, and a 20,000-document definition corpus has 342 mismatches, all of them setext or definition-validity cases such as [ref]: with no destination, none of the reported one.
I also re-measured the correctly-conditioned version of the change this asks for, rather than the unconditional one tried earlier. It removes 37 and 67 mismatches on two corpora but adds 956 on the definition corpus, so it is a net loss and stays out. No change made.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b72fcf440c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return ( | ||
| _PARAGRAPH_TEXT.match(line) is not None | ||
| and _NOT_PARAGRAPH.match(line) is None | ||
| and _SETEXT_UNDERLINE.match(line) is None |
There was a problem hiding this comment.
When a list paragraph is followed immediately by an unindented === line, CommonMark treats that line as lazy paragraph text within the list item. This predicate instead rejects every setext-shaped line as a lazy continuation, so input such as a bullet, ===, and then an item-indented ## 2.0 prematurely closes the list and indexes the nested heading as a real release. The release-notes API can consequently serve example or nested content for a version that has no document-level release heading; the mirrored frontend list tracker has the same classification.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Correct. ## 1.0, - old note, ===, then an item-indented ## 2.0 returned ['1.0', '2.0']; the oracle renders <li>old note ===<h2>2.0</h2></li>, since a setext underline may never be a lazy continuation line, so the item stays open and that heading is nested rather than a release.
_may_be_lazy and the mirrored mayBeLazy no longer reject underline-shaped lines. --- still closes the item through the thematic-break alternative, which I checked separately. The frontend had it too: a bullet, ===, then an indented link left the destination relative and now resolves. Fixed in ae07b5b.
| after_paragraph = False | ||
| continue | ||
| # A dashed underline is not a list marker, so track lists after setext. | ||
| lists = _open_lists(structural, lists, after_paragraph) |
There was a problem hiding this comment.
When an unquoted ordered list follows a blockquote without a blank line, the previous quoted paragraph's after_paragraph state is passed into _open_lists. For example, CommonMark renders > quote, then 2. item, then an item-indented ## 2.0 as a list containing the heading, but this parser decides that an ordered marker starting at 2 cannot interrupt the quoted paragraph, never opens the list, and indexes the nested heading as a real release. Paragraph state needs to be scoped to the blockquote container before classifying the outer line.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Correct. > quote, 2. item, then an item-indented ## 2.0 returned ['1.0', '2.0']; the oracle closes the quote, opens <ol start="2"> and nests the heading inside the item. The same held for > # inner, a bare >, and the lazy route through > quote / text / 2. item.
Paragraph state is now scoped to its container: _open_lists and openLists take a quoted flag, a quote line leaves only the quote's own paragraph open, and both frontend scanners carry the quote state. Port parity re-checked at 27,001 steps with no mismatches. Fixed in ae07b5b.
| content = content[item.end() :] | ||
| # Only ordinary text continues a paragraph. Indented code counts four | ||
| # spaces past the container, so an item's own indent does not count. | ||
| indented_code = not after_paragraph and _indent_width(visible) - column >= 4 |
There was a problem hiding this comment.
If an indented code block is followed without a blank line by an ordered-marker-shaped paragraph starting above 1, CommonMark ends the code block but does not start a list; for example, after four-space-indented code, 2. item is paragraph text and a following three-space ## 2.0 is a document-level heading. Because indented-code state is computed only for the current line, this parser instead opens a list at 2. item and suppresses the real release heading as nested content, causing the popup to report no notes for that version.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This one does not hold, and it is the mirror image of the ordered-marker report from the previous round, which did not hold either. The oracle gives the same answer for both variants:
code -> <pre><code>code</code></pre>
2. item <ol start="2"><li>item<h2>2.0</h2></li></ol>
## 2.0
with or without a blank line before 2. item. The code block ends at the first line not indented enough to continue it, and there is no open paragraph for the marker to be text of, so the list opens whatever the start number, and the three-space heading really is nested. Head already produced ['1.0'] in both cases, so there is nothing to fix.
Last round the claim was that the list wrongly opens; this round it is that the list wrongly does not. Both were checked against the renderer and both are wrong. I added a regression test pinning both variants so this shape stops coming back. No change made.
| String.raw`(!?)\[${NESTED_LABEL}\]\(\s*(<[^<>\n]*>|${BALANCED_DESTINATION}${CLOSES_LINK}|${PLAIN_DESTINATION})`, | ||
| "g", | ||
| ); | ||
| const REFERENCE_TARGET = /^( {0,3}\[((?:[^[\]\\]|\\.)*)\]:\s*)(<[^<>\n]*>|\S+)/; |
There was a problem hiding this comment.
CommonMark permits a link-reference destination on the line after its label, such as [guide]: followed by an indented docs/a.md. The renderer recognizes that as a definition, but this line-scoped expression only rewrites definitions whose destination appears beside the label, leaving the valid multiline form unchanged. References to it therefore navigate relative to the Studio application origin instead of the Unsloth repository; multiline image definitions similarly load from the wrong host.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
The observation is right: a definition whose destination sits on the next line is valid CommonMark and this resolver, which is line scoped, does not rewrite it. I am not taking it here, for the same reason I gave for the earlier multiline-destination report: the rewriter maps every match back to byte offsets on one line, and the code-span, comment, escape and definition rules are all aligned to those offsets, so spanning lines means rewriting that machinery rather than widening one expression. That is a change worth doing deliberately, not as a review fix inside an update-popup PR.
Worth recording what it costs today: the changelog this reads has no link reference definitions at all, single or multi line, so nothing currently ships broken. If one is added, the single-line form is handled and the two-line form is the gap.
| if (inRawHtml) { | ||
| track(""); | ||
| inRawHtml = !RAW_HTML_CLOSE.test(original); | ||
| masked.push(" ".repeat(original.length)); |
There was a problem hiding this comment.
Release notes can use sanitized raw HTML such as <a href="docs/a.md"> or <img src="images/demo.png">, and Streamdown renders those URL-bearing elements. The classifier masks every raw HTML block and the rewriting pass only recognizes Markdown link syntax, so relative href and src values remain relative to Studio's own origin and produce broken links or images. Raw HTML should remain excluded from Markdown parsing while its URL attributes are still resolved against the repository.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
The premise checks out. Streamdown depends on rehype-raw alongside rehype-sanitize and rehype-harden, so sanitized raw HTML does render, and a relative href or src inside it would resolve against Studio's origin.
I am still not taking it here. Resolving URL attributes inside raw HTML means teaching the rewriter to parse HTML attributes and rewrite inside a region it currently masks wholesale, which is a new capability rather than a correction to an existing one, and the masking is load bearing for every other rule in that file. The current changelog contains no raw HTML elements carrying a URL, so nothing ships broken today.
Flagging it as the right follow-up if release notes ever start using raw HTML for links or images, since at that point the masking and the rewriting would need to be reconciled properly rather than patched.
Two lines the parser read as block starts are lazy paragraph text, so the list they were written under closed early and the heading indented to the item's content column was indexed as a release the renderer never shows. A setext underline may never be a lazy continuation line (spec 0.31.2 section 4.3), so `===` written left of an open item is more of that item's paragraph. Rejecting every underline-shaped line ended the list there. A row of three dashes is still a thematic break, which does end it. Lazy continuation runs the other way too: a marker written outside a blockquote is not text of the quote's paragraph, so `2. item` under `> quote` opens a list even though an ordered marker past 1 may not interrupt a paragraph. Paragraph state is now scoped to its container: a quote line leaves open only the quote's own paragraph, an underline needs one in its own container, a definition ends one only when there is none to continue, and a line four columns past its container is code, which may not interrupt. The frontend pair reads the same tracker, so both scanners now carry the quote state and a fence inside a list item ends with the item in the preview the way it already did on the backend. Measured against markdown-it-py (CommonMark 0.31.2) over 264k generated documents: 3368 sections now match the renderer, none regressed, and every list and quote corpus is exact.
for more information, see https://pre-commit.ci
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d5e546119c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // has to resolve to the raw file, not to its page on GitHub. | ||
| const IMAGE_REFERENCE = | ||
| /!\[((?:[^[\]\\]|\\.)*)\](?:\[((?:[^[\]\\]|\\.)*)\]|(?!\())/g; | ||
| const FENCE = /^ {0,3}(`{3,}|~{3,})(.*)$/; |
There was a problem hiding this comment.
When release notes contain a fenced code block inside a blockquote or nested deeply enough in a list, this margin-relative regex never recognizes the opener. Tilde fences—and backtick fences whose valid closer has a different run length—are therefore passed to rewriteLine, so sample text such as [guide](docs/a.md) is silently changed to an absolute GitHub URL in the expanded notes. Detect fences after accounting for the active quote/list container so code examples remain literal.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Correct, and your qualifier is the interesting part. A fence whose opener and closer are equal runs of backticks was protected by accident, because the code-span scanner pairs the two runs across lines and the span guard then blocks the rewrite. Tilde fences and uneven backtick runs had nothing holding them, so > ~~~ / > [guide](docs/a.md) / > ~~~ was rewritten on head while the renderer shows it verbatim.
Blocks are now detected against the container content rather than the margin, using the tracker the backend and preview already share. One thing the report did not cover was needed too: once these blocks correctly open inside a container, they need the same container scoping a fence already had, so the fence column became a block column covering all three kinds. Without that a <pre> opened inside an item swallowed everything below it and the container corpus regressed 69 documents; with it, one. Fixed in 13db203, together with the HTML-block half.
| // Type 6 and 7 blocks are literal too and run to the next blank line, not to a | ||
| // closing tag, so `<details>` holds Markdown only after a blank line. Type 7 | ||
| // (any other complete tag alone on a line) cannot interrupt a paragraph. | ||
| const HTML_BLOCK_OPEN = /^ {0,3}<\/?([a-zA-Z][a-zA-Z0-9-]*)(?=[\s/>]|$)/; |
There was a problem hiding this comment.
When a raw HTML block is nested at four or more source columns, such as a <details> block under a nested bullet, this margin-relative pattern misses the opener even though CommonMark measures its indentation from the list container. classify then treats the block body as Markdown and rewriteLine changes literal examples such as [guide](docs/a.md) into absolute URLs. Detect HTML blocks relative to the active container so expanded release notes do not silently mutate their contents.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Correct, and it is the same fix as the fence one, since both were matching against the margin instead of the container. Five of seven probe cases were wrong on head, including a <details> under a nested bullet and a <pre> inside a blockquote, both of which the renderer keeps literal. Comments were already safe, because the comment masker blanks an inline <!-- ... --> wherever it sits. Fixed in 13db203.
| continue; | ||
| } | ||
| const explicit = match[2] ?? ""; | ||
| imageLabels.add(label(explicit.trim() ? explicit : (match[1] ?? ""))); |
There was a problem hiding this comment.
When one reference label is used by both an image and a regular link, adding that label to imageLabels forces their single definition onto IMAGE_BASE. For example, ![preview][asset] followed by [read guide][asset] makes the latter open the raw-content URL instead of the intended GitHub page. CommonMark permits definitions to be shared this way, so resolve each usage according to whether it is an image rather than assigning one base to the definition globally.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
The divergence is real and reproduces exactly as filed: with ![preview][asset], [read guide][asset] and one shared definition, head puts the definition on the raw host while the renderer wants the image raw and the link on the blob page.
I am not taking it here. One definition line cannot carry two bases, so deciding per usage means inlining every reference usage with its title, which needs a link-reference scanner and either merges the image and link matchers or adds a second pass. The inline matcher's offsets are used against document offsets for the code-span guard, so a first pass that changes line lengths breaks that guard on the main path; this is a rewrite of the core matcher rather than a targeted fix.
On the cost of leaving it: for the image to be an image its destination is an image file, so the shared link opens the raw PNG instead of its blob page, which still shows the image. Of the two single-base outcomes the current one is the safer, since a blob URL as an img src is a broken image while a raw URL as a link is not a broken link. Worth doing properly if reference-heavy notes ever arrive, not as a review fix here.
| // parentheses when they balance, so `[x]((draft).md)` points at `(draft).md`; | ||
| // one nesting level is all a file name needs, as in the preview's DESTINATION. | ||
| const NESTED_LABEL = String.raw`((?:[^[\]\\]|\\.|\[(?:[^[\]\\]|\\.)*\])*)`; | ||
| const BALANCED_DESTINATION = String.raw`(?:\\.|[^\s()]|\((?:\\.|[^\s()])*\))*`; |
There was a problem hiding this comment.
A valid destination that begins with more than one nested parenthesis, such as [x](((draft)).md), cannot be consumed by this one-level expression, so the resolver leaves the relative target unchanged and Studio resolves it against its own origin. CommonMark accepts balanced nested parentheses in destinations; parse the complete balanced target so repository links with such filenames still point back to GitHub.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Correct. [x](((draft)).md) was left relative on head and the renderer resolves it, as it does ).png) and the title form.
The nesting is unrolled to 32 levels, the depth cmark stops counting at. The unrolled form is linear rather than exponential: each level references the previous once, and at any position exactly one alternative can start, so there is no ambiguous backtracking. Adversarial benches confirm it, with 33-deep opens and 32-deep balanced destinations at 8 ms and 23 ms for 4,000 repetitions and a single 100k-open destination at 1 ms.
The guarantee from the earlier round is intact: [x](a(b.md) is still not a link and still falls to the paren-free expression byte for byte, and a balanced opener does not swallow across a line break. Paren corpus over 20,000 documents: 7,286 fixed, 0 regressed. Fixed in 13db203.
A block is measured from its container and not from the left margin (spec 0.31.2 sections 4.5 and 5.2), but the link resolver's fence, raw HTML and type 6 expressions all started at the margin, so a fence behind a quote marker and one three columns under a nested bullet opened nothing. The sample inside was then read as prose, and a relative link written in a code block or a details body was rewritten into text the reader is shown verbatim. Matching runs of backticks hid some of it by accident, since the code span scanner pairs them across lines, but a tilde fence, a closer of a different length and every HTML block went through. Each line is now read from the container it is written in, which the list tracker already knew, and a block is scoped to that container the way a fence inside an item already was: a line to the left of the item, or outside the quote, ends the block along with it, and a bare quote marker is the blank line that ends a type 6 block. A destination holds parentheses while they balance, and a path may nest them, so [x](((draft)).md) points at ((draft)).md. One nesting level was all the expression allowed, so anything deeper fell through to the plain form, matched an empty destination and left the link resolving against Studio's own origin. The pairs are unrolled to the 32 levels cmark counts, and the balanced form is still gated on a closer following it, so the stray paren in [x](a(b.md) stays the closer the way CommonMark reads it rather than inventing a link across lines. Measured against markdown-it-py (CommonMark 0.31.2) over 66k generated documents, comparing the rendered HTML rather than the destinations alone: 7286 documents in the parenthesis corpus and 313 in the container corpus now match the renderer, and the link and definition corpora are unchanged. One container document regresses, where closing the HTML block correctly exposes an unrelated gap of its own: a link reference definition still leaves a paragraph open, so the indented line below it reads as prose rather than as code. The list tracker still matches the backend on every step, the repo's own CHANGELOG resolves identically, and the pathological inputs measure the same.
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6363f69f88
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const spans = [...codeSpans(masked), ...comments].sort( | ||
| (a, b) => a.start - b.start, | ||
| ); |
There was a problem hiding this comment.
When a multiline code span contains an inline comment before a relative link, e.g. `code\nx <!-- hidden -->\n[guide](docs/a.md)`, comments produces ranges that sit inside the global code span. Sorting the overlapping ranges breaks insideSpan's binary search assumption, so it can skip past the outer code span and rewriteLine rewrites the link even though it renders as literal code; merge overlapping ranges or check code spans before comment ranges.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
The overlap is real, the trigger is not. Built the exact document (`code, x <!-- hidden -->, [guide](docs/a.md)`): markdown-it-py makes it one code span and head rewrites the destination inside it, so the ranges do break the binary search. But it needs a code span opened on one line and closed two or more lines later with an HTML comment on an interior line, which is what puts comment ranges inside the outer span. Single line spans are safe, since maskInline already skips openers inside per-line code spans. CHANGELOG.md holds no multiline code spans, so this is below the bar for a change here.
| } | ||
| text.push(index); | ||
| masked.push(line); | ||
| afterParagraph = !blank && !BLOCK_LINE.test(line); |
There was a problem hiding this comment.
When one reference definition immediately follows another, the first definition reaches this assignment and sets afterParagraph to true. The next [two]: docs/two.md line is then omitted from definition, so its relative destination is left unchanged even though the Markdown renderer still uses it for [two], causing that reference to resolve against Studio instead of the repository; reference definitions should not open paragraph state.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed, and reachable on an ordinary section. [amd]: docs/basics/amd.md immediately followed by [xpu]: docs/basics/xpu.md, both used from a bullet above: markdown-it-py resolves both against the repo base, head rewrites only the first and leaves [xpu] relative, so that reference resolves against the Studio origin instead. The first definition line reaches the final afterParagraph assignment in classify, so the second never enters the definition set. Consecutive definitions with no blank line between them are valid per CommonMark 0.31.2 section 4.7, and the backend already guards this in changelog.py with (after_paragraph or _LINK_DEFINITION.match(visible) is None), so this is also a parity gap against it.
There was a problem hiding this comment.
Confirmed and fixed in 4fcc9de. classify() set afterParagraph on a definition line, so a second definition never entered the definition set and its destination stayed relative, resolving against the Studio origin. Reproduced with the two-definition bullet: markdown-it-py resolves both, head rewrote only the first. The fix is the frontend spelling of the guard changelog.py already carries, (after_paragraph or _LINK_DEFINITION.match(visible) is None), rather than a new rule. Regression test test_one_definition_does_not_hide_the_next covers two and three definition runs, and prose between two definitions still opens a paragraph the next line may not interrupt.
| const BLOCK_LINE = | ||
| /^ {0,3}(?:#{1,6}([ \t]|$)|(?:\*[ \t]*){3,}$|(?:-[ \t]*){3,}$|(?:_[ \t]*){3,}$|>|=+[ \t]*$)/; |
There was a problem hiding this comment.
When a release body uses a valid one- or two-hyphen setext h2 before reference definitions, such as Overview\n--\n[guide]: docs/a.md, the underline renders as a heading and leaves no paragraph open. This block test only recognizes three-or-more hyphens, so afterParagraph stays true and the following definition is not rewritten; any [guide] reference then resolves relative to Studio instead of the repository.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
The spec point is right, and it is a parity gap against the backend, which matches _SETEXT_UNDERLINE = ^ {0,3}(=+|-+)[ \t]*$ at any length. But I could not reach it on real input. Overview / -- / [guide]: docs/a.md plus a [guide][guide] use: markdown-it-py makes -- an h2 and resolves the definition, head leaves it relative. Same with a single -, and the same shape over indented code rewrites a link the renderer shows as code. Three or more hyphens already work through the thematic break arm of BLOCK_LINE and == through the =+ arm, so the gap is exactly a one or two hyphen underline with a definition or indented code under it. CHANGELOG.md documents ATX headings for releases and uses them throughout, so leaving BLOCK_LINE as is for now.
| return { columns, emptyItem: false }; | ||
| } | ||
| const marker = item[1] ?? ""; | ||
| let padding = indentWidth(item[2] ?? ""); |
There was a problem hiding this comment.
When a list marker is followed by a tab, such as -\tDetails, Markdown expands that tab from the column after the marker, so the item content starts at column 4. Measuring item[2] from column 0 records column 5 instead; after a blank line, a four-space continuation like [guide](docs/a.md) is classified as document-level indented code and its relative link is left pointing at Studio rather than the repository.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
The off by one is real, the trigger is not reachable. -\tDetails, blank line, [guide](docs/a.md): markdown-it-py puts the item content column at 4 and renders the link inside the item, head records 5, closes the item on the 4 column line and reads it as document level indented code, so the link stays relative. The same document with - Details is correct. It needs a tab immediately after a list marker plus content at the item content column, and every list in CHANGELOG.md uses a space marker with two space continuations, so not changing openLists for it.
…its paragraph Four things the three changelog scanners read differently from a renderer. A fence written straight after a list marker is the item's own first content, measured from the column that content starts, so "- ```md" opens one. All three scanners matched the whole line and saw nothing, so the code sample below it was prose: the resolver rewrote a destination the reader sees verbatim, and the preview offered the info string as a headline bullet. A shared itemContent / _item_content reads past a marker that really opens an item, capping the padding the way the list tracker caps it so an over-indented line is still indented code. An HTML block opener is read the same way, and its marker survives into the structural line so the item it opens is still tracked. An HTML block holds no lazy continuation line, so one opened on an item's continuation line ends where the item does, exactly as a fence there already did. The backend and the preview ended it only on a blank line, so it ran past the item and swallowed the next release heading, which made those notes unreachable and dropped every bullet below it from the collapsed popup. A raw block inside an item ends on a blank line too, which is where cmark puts it. A comment written mid-sentence is inline raw HTML belonging to the paragraph around it, so its "-->" may arrive on a later line of that same paragraph. Ending it at its own line left a backtick inside it pairing with a real one below, which hid a following link from the resolver, and left the preview quoting text the popup body does not show. A shared commentClosesBelow answers whether the closer arrives before the paragraph breaks; where it does not, the opener stays the ordinary text a renderer shows, so a note that merely mentions "<!--" still hides nothing. Only ASCII punctuation is escapable, so the backslash in "docs\alpha.md" is a character of the path. Dropping every backslash rewrote it to a path that does not exist, and a URL parser reads what survives as a separator, so a Windows or namespaced path pointed at the wrong file either way. The destination expression now escapes only punctuation, which also means a space still ends a destination: "[x](a b.md)" and "[x](a(b.md)" are not links, so their paths are left alone rather than half-rewritten. A destination that runs out of line still resolves, since its closer is on the line below. Fuzzed against markdown-it (CommonMark 0.31.2) over 20k-document corpora, with the whole rewritten document rendered and compared, not just its destinations. Release headings: 117 to 16 on containers, 88 to 10 on markers, 17 to 12, nothing new anywhere. Link destinations: 8823 to 104 on markers, 114 to 98 on comments, nothing new. Whole-document renders: 9271 to 220, 5116 to 245, 1265 to 671. The Python and TypeScript list trackers still agree over 26861 steps, and itemContent and hiddenStructure agree over another 6335. 321 KB of unmatched backticks still measures the same.
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 52b2f2c8b0
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const MAY_START_BLOCK = | ||
| /^[ \t]*(?:$|[<>=*+_|-]|`{3,}|~{3,}|#{1,6}([ \t]|$)|\d{1,9}[.)]([ \t]|$))/; |
There was a problem hiding this comment.
The new shared block-start guard is still too broad for inline comments: when the next paragraph line starts with a literal punctuation character that is not actually block syntax, such as Note <!-- hidden\n-still hidden -->, CommonMark keeps that line in the same paragraph and closes the inline comment. This pattern marks any leading -, _, |, etc. as a block break, so the scanners leave the opener/comment body visible; the collapsed preview can expose hidden TODO text and the link resolver can rewrite links inside content that the renderer hides.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed, and the visible damage is in the collapsed preview. For
- DoRA training is available in Studio. <!-- TODO confirm the exact
flag name before release
-->
markdown-it-py parses one inline raw HTML comment and renders only "DoRA training is available in Studio."; releaseNotesPreview at head emits DoRA training is available in Studio. <!-- TODO confirm the exact flag name before release --> as the bullet, so an authoring note ships in the update popup. A --> alone on its own line, which is the usual way to write a multiline comment, hits the leading - arm of MAY_START_BLOCK, so commentClosesBelow returns false and both scanners keep the opener visible. Same for a continuation starting *before* release --> or _draft_ note -->. A continuation starting with a letter is handled correctly.
There was a problem hiding this comment.
Confirmed and fixed in 4fcc9de. MAY_START_BLOCK read any line whose first non-space character was one of a punctuation class as a block break, so a closing arrow on its own line, the ordinary way to write a multiline comment, made commentClosesBelow report false and the whole comment stayed visible in the popup. That character class is now startsBlock(), which tests for a block that can actually interrupt a paragraph and reuses interruptsParagraph from markdown-list-columns rather than duplicating the blockquote and list rules a fourth time. Continuations starting with emphasis are covered too. A leading angle bracket is still read as a break, so no fourth copy of the HTML block tag table was needed and head behaviour there is unchanged.
| // is only part of the link. One that stops short of a closer is no destination | ||
| // at all, so `[x](a b.md)` and `[x](a(b.md)` are the plain text they render as | ||
| // and keep the paths they name. | ||
| const CLOSES_OR_ENDS_LINE = String.raw`(?=[ \t]*(?:[)'"]|$))`; |
There was a problem hiding this comment.
When a valid inline link uses the parenthesized title form, for example [guide](docs/a.md (Guide)), CommonMark still renders docs/a.md as the destination and (Guide) as the title. This lookahead only accepts ), ', or " after the destination, so the resolver does not match that link at all and the relative URL remains pointed at the Studio origin instead of the repository.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Reproduces, but not on input this file sees. [guide](docs/a.md (Guide)): markdown-it-py gives destination docs/a.md and title Guide, head leaves the link untouched because neither CLOSES_LINK nor CLOSES_OR_ENDS_LINE accepts ( after the destination. [guide](docs/a.md "Guide") and the single quote form both rewrite correctly. CHANGELOG.md carries no link titles at all and the parenthesised form is not used in practice, so leaving the lookahead as is.
| // Scanned over the whole document, so a span may cross a line break. | ||
| // Commented ranges join them: the renderer shows neither, so a link in one | ||
| // is not followable and rewriting it would only mutate hidden text. | ||
| const spans = [...codeSpans(masked), ...comments].sort( |
There was a problem hiding this comment.
When two separate paragraphs each contain an unmatched backtick and a relative link sits between them, such as Use \ herefollowed by a blank line and thenSee guide and ` there, Markdown parses the backticks as literal text and renders the link. Scanning code spans over the whole document pairs those backticks across the paragraph break, so insideSpan` suppresses rewriting the rendered link and it navigates relative to Studio rather than GitHub.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed as a divergence, and it is worse than described: one stray backtick is enough, since it pairs with the opening tick of ordinary later inline code. A ` tick / blank / - See [x](docs/a.md) and `code` : markdown-it-py renders three separate blocks with docs/a.md a live link, head returns the document byte identical because the document wide span swallows it. Order matters, a stray after the link is harmless. Rejecting on reachability only: it still needs an unmatched backtick, and CHANGELOG.md plus all 46 release bodies have none. Worth fixing when this file is next touched, since codeSpans(masked) at line 595 is the only scanner that pairs across a blank line, but not blocking here.
|
|
||
| // Inline `](dest)` plus the `[label]: dest` reference form. The destination is | ||
| // either <bracketed> or runs to whitespace or the closing paren. | ||
| const NESTED_LABEL = String.raw`((?:[^[\]\\]|\\.|\[(?:[^[\]\\]|\\.)*\])*)`; |
There was a problem hiding this comment.
When rendered link text contains more than one nested bracket pair, such as [outer [inner [deep]]](docs/a.md), CommonMark still treats the following destination as the link target. This label pattern only allows one bracket level, so the resolver leaves those valid links unchanged and the relative URL is followed from Studio's origin instead of the repository.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Reproduces at two levels of nesting only. [outer [inner [deep]]](docs/a.md): markdown-it-py resolves it, head leaves it relative. [outer [inner]](docs/a.md) and the badge shape [](docs/a.md) both rewrite correctly, and several one level pairs in the same label are fine because NESTED_LABEL repeats the alternation. Link text nested two brackets deep does not occur in release notes, so this is below the bar.
| const resumed = closed + COMMENT_CLOSE.length; | ||
| return maskInline(line, resumed, closesBelow); | ||
| } | ||
| if (COMMENT_BLOCK_OPEN.test(line)) { |
There was a problem hiding this comment.
When a list item starts with an HTML comment, for example - <!-- hidden followed by an indented [guide](docs/a.md) and -->, CommonMark reads the comment opener from the item content column and hides the whole block. This check runs on the raw line before removing the list marker, so the scanners treat it as visible inline text: hidden links get rewritten in the expanded notes and the collapsed preview can show the comment body.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed, and it bites on a single line, no multiline comment needed. - <!-- new --> AMD support, see [the guide](docs/amd.md): because the opener is the item's first content, CommonMark makes the whole line a type 2 HTML block, so markdown-it-py renders the literal text AMD support, see [the guide](docs/amd.md) and no link. Head misses the block because COMMENT_BLOCK_OPEN reads the raw line, hides only <!-- new --> inline, and rewrites the destination, so with Streamdown's rehype-raw the popup shows the absolute URL as literal text. releaseNotesPreview compounds it, previewing AMD support, see the guide as if the bullet were Markdown. Also reproduces with - <!-- hidden / [a](docs/x.md), and with *, 1. and nested markers. Rewriting my earlier read: this is independent of the MAY_START_BLOCK issue, the same shapes diverge with that neutralised. The doc-level form <!-- x --> [a](docs/b.md) and a mid-bullet - Fixed X <!-- ref --> see [g](docs/b.md) are both handled correctly.
There was a problem hiding this comment.
Confirmed and fixed in 4fcc9de. COMMENT_BLOCK_OPEN tested the raw line, so the opener was never seen as the item first content and the destination inside a type 2 HTML block was rewritten, which Streamdown then shows as a literal URL. It now tests itemContent(container, afterParagraph) in all three scanners. Two things came with it: the marker has to survive the mask, or the item loses its content column and a nested heading escapes the item, and the comment block has to be scoped to its item alongside fences and raw blocks. The backend needed the same change, and it was reachable: a bullet opening a comment followed by an indented heading made parse_changelog index a release CommonMark does not. Differential fuzz over 16000 documents per side shows 107 backend divergences fixed and none introduced.
| // and keep the paths they name. | ||
| const CLOSES_OR_ENDS_LINE = String.raw`(?=[ \t]*(?:[)'"]|$))`; | ||
| const INLINE_TARGET = new RegExp( | ||
| String.raw`(!?)\[${NESTED_LABEL}\]\(\s*(<[^<>\n]*>|${BALANCED_DESTINATION}${CLOSES_LINK}|${PLAIN_DESTINATION}${CLOSES_OR_ENDS_LINE})`, |
There was a problem hiding this comment.
When an angle-bracketed destination contains an escaped >, such as [x](<docs/a\>b.md>), CommonMark treats it as a link to docs/a>b.md. The angle-destination arm stops at that escaped delimiter and rewrites only the prefix, leaving the rest of the filename outside the URL and producing malformed Markdown instead of a repository link to the intended path.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Reproduces, and the output is worse than described: [x](<docs/a\>b.md>) comes back as [x](<https://github.com/unslothai/unsloth/blob/main/docs/a%5C>b.md>), which markdown-it-py then reads as literal text plus an autolink rather than one link. [x](<docs/a\<b.md>) is handled correctly. Reaching it needs a repository path containing >, which cannot exist on Windows and does not exist here, so leaving the angle destination arm alone.
…m it opens in Three CommonMark conformance fixes in the changelog scanners. A link reference definition is a block of its own that may not interrupt a paragraph, so it opens none either: definitions are allowed to run consecutively (spec 0.31.2 section 4.7). The link resolver counted one as paragraph text, so every definition after the first fell outside the set of lines a definition may start on and kept its relative destination, which then resolved against Studio's own origin. The backend already read the line this way. The guard asking whether a `-->` is reachable from an opener read any line whose first character was punctuation as the start of a new block. A `-->` written on a line of its own is how a multiline comment is ordinarily closed, and a wrapped line may open with emphasis, so neither counted as more of the paragraph carrying the comment. The comment never closed and the collapsed popup showed the author's internal note to the reader. It now tests for a block that may actually interrupt a paragraph. A comment is an HTML block too (section 4.6, type 2), so one written as a list item's first content opens inside that item exactly as a fence written there does. All three scanners looked for the opener at the margin of the line as written, so a marker in front of it hid the block: the resolver rewrote a destination inside raw HTML, which Streamdown then shows the reader as a literal URL, and the preview quoted the hidden note back at them as though the bullet were Markdown. The opener is now read from the item's content, the marker survives into the structural line so the item it opens is still tracked, and the block is scoped to that item the way a fence there is.
for more information, see https://pre-commit.ci
|
@codex review |
|
Codex Review: Didn't find any major issues. Another round soon, please! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review |
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
Ran this end to end rather than reading it: installed the branch at 6579491 into an isolated Matched notes, collapsed
The first sentence of each bullet is emphasised and the rest dimmed and clamped, so the change reads first and the detail recedes. Expanded, with nested content
Sub items keep their own indentation, inline code renders as code, and fenced blocks are highlighted. Relative links resolve against the repository
The source was A version with no section
Offering 2026.9.9 with 2026.8.0 and 2026.7.6 sitting next to it in the same file. Collapsed the panel renders nothing at all; expanded it says so plainly. No notes borrowed from an adjacent release. One finding worth a look
With the default sources, that "no notes yet" state is currently unreachable. Two separable halves. The 404 half self heals once this merges and main has the file. The conflation does not: a 404 or 403 is a well formed HTTP answer rather than unreachability, yet it is worded as "Could not reach", cached for only the 5 minute failure TTL, refetched forever, and given a Retry that cannot succeed. The same applies to anyone behind a proxy that 403s raw.githubusercontent. Distinguishing Two smaller notes. Links render as What was faked, stated plainly: |
3 participants
The update banner told you a new version exists and linked out to the online changelog, so there was no way to see what an update contains before taking it. This adds the notes to the popup itself.
Where the notes come from
CHANGELOG.mdat the repo root is the source. Studio reads it from the default branch, so editing the file here updates the popup for everyone on the next update check, with no release or rebuild needed. A copy is bundled into the wheel at build time as an offline fallback, and the root file stays the only one to edit.Every release is a level-2 heading whose first token is the version:
## 2026.7.6 - 2026-07-22## [2026.7.6] - 2026-07-22and## v2026.7.6also parse.## Unreleasedand other non-version headings end a section but are never indexed, and a##inside a fenced code block is treated as sample markdown rather than a heading.Notes are pinned to one exact version
This was the main thing to get right. The popup asks for the version it is offering and gets that section or nothing. Matching is version-aware, so
2026.07.6and2026.7.6are the same release, but it is never fuzzy: a near miss returns no notes rather than the closest section. If a version has no section yet, the popup says so and links out to the online changelog instead of showing notes from an unrelated release. The expanded state is keyed by version in both banners too, so a newly offered version collapses the panel rather than leaving the previous release's notes on screen.UI
Collapsed, the popup previews the top four bullets with each bullet's leading sentence highlighted and the rest dimmed. "Show release notes" expands the full notes as rendered Markdown in a scrollable panel, with the scroll thumb hidden until hover via the existing
hover-scrollbarutility.Both banners are covered. The desktop updater already fetched the GitHub release body and never displayed it; that body is now the fallback when
CHANGELOG.mdhas no section for the offered version.The card is 448px rather than 400px so the notes toggle sits inline with the other two buttons at the same type size. Width previously lived on the shared bottom-right overlay stack, so it moved onto each overlay to avoid widening the llama.cpp banner and the download panel along with it.
Backend
GET /api/studio/release-notes?version=Xreturns the section for that version. Authed, 422 on a malformed version, 30 minute cache, 3s timeout, 2MB cap, and no network call at all whenUNSLOTH_DISABLE_UPDATE_CHECK=1.Testing
tests/studio/test_update_release_notes.py, 21 tests. Exact-version matching and the near-miss cases, section boundaries, fenced-code and non-version headings, remote winning over the bundled copy (against a local HTTP server), rejected version queries, and the frontend contracts for the scroller, the preview and the toggle placement.Also exercised the route end to end:
2026.7.5returns matched notes,2026.9.9returnsmatched: falsewith null markdown, and../etc/passwdreturns 422.One thing to flag
The popup only shows for PyPI installs, so it cannot be triggered from a source checkout. I added
UNSLOTH_STUDIO_FAKE_UPDATE=<version>inupdate_status.pyto offer a given version locally for review. It is how this was tested. Happy to drop it if we would rather not carry it.