fix(workspace): resolve a figure whose alt text holds balanced brackets - #869
Open
ANIRUDDHA ADAK (aniruddhaadak80) wants to merge 1 commit into
Conversation
The alt-text scan stopped at the first unescaped `]`, so `![Figure [a] b](figures/result.png)` was never read as an image: the next character was a space rather than `(` or `[`, no reference label matched, and the manuscript preview kept the relative URL instead of the file route, leaving the figure broken. An image description may hold balanced brackets, so the scan now counts them with the same `escaped()` helper and `destination()` depth shape it already uses for parentheses; the first `]` outside every unescaped pair closes the alt text, and an unbalanced `]` still ends it. A nested image in the alt text no longer shadows the image that contains it. Covered by a case in frontend/workspace/src/manuscript/model.test.ts, which fails against the previous scan.
ANIRUDDHA ADAK (aniruddhaadak80)
requested review from
Aayam Bansal (aayambansal) and
Ishaan Gangwani (ishaan1124)
as code owners
September 30, 2026 10:14
|
ANIRUDDHA ADAK (@aniruddhaadak80) is attempting to deploy a commit to the InkVell Team on Vercel. A member of the Team first needs to authorize it. |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do, and why?
A manuscript figure whose alt text contains brackets never rendered in the preview.
The image scanner that rewrites relative image URLs to the file route ended the alt text at the first unescaped
], with no bracket-depth tracking:CommonMark allows balanced brackets inside link text, so
![Figure [a] b](figures/result.png)is an image with altFigure [a] b. This scanner took the alt to beFigure [a, saw that the next character was a space rather than(or[, failed to resolve a reference definition, and never recorded the image at all.markdownImagesreturned[],rewritePreviewImagesleft the relative path exactly as written, and the figure stayed broken. A nested image,](y.png), was attributed toi.pnginstead ofy.png.The alt scan is now bracket-depth aware, reusing the same
escaped()helper and depth idiom the destination parser in this file already uses. Unbalanced brackets are still not images, per the spec.Linked issue
small fix, no issue
How did you verify it?
Before the fix:
After the fix:
Cross-checked against CommonMark 0.31.2 §6.4 (image descriptions follow the link-text rules, which "may contain balanced brackets, but not unbalanced ones, unless they are escaped"; example 520
](uri2)](uri3)resolves touri3).Tests and gates:
cd frontend/workspace && bun test --timeout 15000 src/manuscript/model.test.ts— 9 pass, 0 fail (1 new test, 3 fixtures)bun test --timeout 15000 src/manuscript/model.test.ts src/utils/markdown-assets.test.ts src/utils/markdown-file-links.test.ts src/atlas/FilePreviewMarkdown.test.ts— 29 pass, 0 failtooling/util/src/markdown.tsreverted and the new test left in place, 8 pass / 1 fail, and the single failure is the new case. The eight pre-existing cases — inline, local, remote and data images, titled and angle-bracketed destinations, balanced parens in the destination, reference-style image plus its definition, backtick span, fenced block, indented code block, CRLF offsets — all still pass, so the fix does not regress what already worked.bun run --cwd tooling/util typecheck— exit 0All matched files use Prettier code style!Where the test lives, and why: the new case went into the existing
frontend/workspace/src/manuscript/model.test.tsrather than a new test file undertooling/util.tooling/util/package.jsonhas only atypecheckscript, the rootpackage.jsonhas notest:util, andowner("tooling/util/src/markdown.ts")intooling/repo/test-shards.tsisundefined— so a test placed there is executed by nothing in Fast CI or Deep CI. The workspace file already covers this parser throughrewritePreviewImagesand runs in Fast CI's workspace step on every PR.Not run locally:
bun run checkin full, the aggregatebun run test:workspace, and Deep CI.bun run --cwd frontend/workspace buildalso fails in this Windows worktree, atfrontend/ui/src/components/icon.tsx(esbuild: "The JSX import source cannot be set without also enabling React's automatic JSX transform"), which is a pre-existing consequence of the@synsci/*packages being resolved from outside the worktree and is unrelated to the two files this PR touches.Checklist
bun run checkis green (format, typecheck, backend + frontend/ui + SDK tests) — ran the workspace tests covering the change, thetooling/utiltypecheck and the format check; the fullbun run checkaggregate and the aggregatebun run test:workspacewere not run locallybun run --cwd frontend/workspace buildsucceeds if I touchedfrontend/workspaceorfrontend/ui— attempted; it fails in this worktree onfrontend/ui/src/components/icon.tsxfor an unrelated pre-existing reason (see above)./tooling/repo/generate.tswas run and thetooling/sdkoutput committed if I changedbackend/cli/src/server— not applicablefrontend/docs/src/content/openscience/is updated if behavior changed — no documented behaviour changes, markdown figures that already worked are unaffectedpackage.jsonversions and tags are written by the release workflow)installandfrontend/landing/public/installare still byte-identical if I touched either — not applicable