Merge main to stable#3321
Conversation
* ci: auto-resolve version conflicts in promote-stable workflow * ci: also match root-level release artifact files in auto-resolve
…#3200) * fix(mcp): fall back to z.unknown() for oneOf with non-object variants z.looseObject({}) emits type:"object" which is right for object-only unions but rejects arrays/booleans/etc. at runtime when the union includes non-object variants. Gate the looseObject path on "every variant is type:object" and fall back to z.unknown() otherwise. The only catalog field this affects today is superdoc_edit.content (oneOf object|array), where the array form was getting rejected before reaching DocumentApi. Adds a unit test that walks the catalog and checks the emitted type for both branches. * docs(mcp): tighten oneOf branch comment with AIDEV-NOTE anchor
Strengthens the existing reminder so agents look for comment-policy.md explicitly and run /comment-audit to validate changes, instead of relying on the bare 'follow comment-policy.md' wording.
* feat(contracts): add typed direction context types (SD-2775)
Introduces orthogonal direction context types so future RTL work cannot
accidentally collapse axes that ECMA-376 keeps separate:
- BaseDirection, WritingMode (enums)
- SectionDirectionContext (page direction, gutter — chrome only)
- TableDirectionContext (visual cell ordering only)
- CellDirectionContext (cell writing mode)
- ParagraphDirectionContext (paragraph inline base direction + writing mode)
- RunBidiContext, RunScriptContext (run-level signals; consumed in 1b/1c)
Adds `directionContext` field to ParagraphAttrs alongside the existing
`direction` scalar. Both are populated by pm-adapter from the same source;
consumers can migrate gradually.
Per ECMA-376 §17.6.1 / §17.3.1.6 / §17.4.1 / §17.3.1.41, each axis stays
separate: section bidi is chrome only, paragraph bidi is paragraph-local,
table visual direction is cell ordering, writing mode is the one
inheriting axis.
No behavior change. Resolver chain and migration follow in subsequent
commits.
* feat(pm-adapter): add direction resolver module (SD-2776)
New module pm-adapter/src/direction/ with:
- resolveSectionDirection / resolveTableDirection / resolveCellDirection /
resolveParagraphDirection — context propagation chain mirroring OOXML
containment hierarchy
- logicalSides helpers (resolveLogicalAlignment, resolveLogicalIndent,
physicalSide, isRtl, toBaseDirection) — direction-aware logical→physical
mapping
- 12 non-collapse tests enforcing the four ECMA spec rules:
1. Section w:bidi MUST NOT make paragraphs RTL (§17.6.1)
2. Table w:bidiVisual MUST NOT make cell paragraphs RTL (§17.4.1)
3. Run-level w:rtl MUST NOT bubble up to paragraph
4. Paragraph w:bidi DOES produce paragraph RTL (§17.3.1.6, including
style cascade through docDefaults per §17.7.2)
- Writing mode IS the one inheriting axis (§17.3.1.41) — paragraph→cell→
section→default
Co-located README documenting the spec rules, a worked-example for
downstream consumers, and explicit non-goals (script classifier and bidi
controls deferred to Wave 1b / 1c).
No production call sites consume the resolver yet; migration follows.
* refactor(pm-adapter): migrate computeParagraphAttrs to direction resolver
Replaces the cascade in resolveEffectiveParagraphDirection +
inferDirectionFromRuns with the typed resolver chain from
pm-adapter/src/direction/. The cascade had three issues identified by
the audit at .tmp/rtl-audit-findings.md:
1. Section→paragraph fallback (§17.6.1 violation) — section bidi
propagated to paragraph inline direction. Latin paragraphs in RTL
sections rendered right-aligned; Word renders them left-aligned.
2. Majority-of-runs heuristic (UAX #9 P2/P3 disagreement) — base
direction came from counting runs whose w:rtl flag was set, not
the first strong character of the text content.
3. docDefaultsDirection parameter (redundant) — the style-engine
cascade in style-engine/src/ooxml/index.ts:165 already resolves
docDefaults.paragraphProperties.rightToLeft into the paragraph's
resolved properties before this resolver runs.
Now: paragraph direction comes from paragraph w:bidi (or its style
cascade); when absent, inlineDirection is undefined and the browser
applies UBA via the missing dir attribute. Output corrected for
documents that today render incorrectly; unchanged for documents that
were already correct.
Tests updated:
- paragraph.test.ts: removed cascade/heuristic tests that codified
the spec violations
- paragraph.test.ts: section-fallback test flipped to assert no
inheritance
- index.test.ts: two integration tests flipped to expect undefined
paragraph direction when only section bidi is set
Validation:
- 1,765 pm-adapter unit tests pass
- 211 contracts unit tests pass
- 12,374 super-editor unit tests pass (incl. footer w:rtl roundtrip)
- 51 RTL Playwright behavior tests pass across Chromium/Firefox/WebKit
Closes SD-2776, SD-2778. The legacy attrs.direction scalar remains
populated for backwards compatibility; consumers should migrate to
attrs.directionContext over time.
* fix(direction): accept rightToLeft on TablePropertiesLike
The resolved TableProperties type from the style-engine uses
`rightToLeft` for the bidiVisual flag (matching the existing importer
convention). The resolver previously checked only `bidiVisual`, so
passing real resolved table properties would leave visualDirection
undefined for RTL tables.
Now accepts both `rightToLeft` (style-engine name) and `bidiVisual`
(OOXML name) for safety. Test added to cover the alias.
* fix(direction): map all ST_TextDirection values incl. V-suffix variants
Per ECMA-376 §17.18.93, ST_TextDirection has 12 enumeration values across
the strict and Word-transitional vocabularies. The V-suffix variants are
glyph rotation, which CSS expresses through text-orientation, so they share
the writing-mode of their non-V sibling.
Before this commit the three resolvers (paragraph/section/cell) handled 6
of the 12 values; lrTbV, tbRlV, tbV, lrV, rlV all fell through to undefined
and the resolver silently used the inherited/default writing-mode instead.
The repo's ST_TEXT_DIRECTION contract (registry.ts:18) publishes lrTbV and
tbRlV as accepted values, so this was a contract violation - documents that
imported one of these would lose their writing-mode override.
Adds an exhaustive test that exercises all 12 values on paragraph, section,
and cell.
* fix(layout-bridge): separate text direction from RTL hit testing
* fix(layout-bridge): harden isRtlBlock and anchor compat-fallback rule
Three review-driven nits on the SD-2780 hit-test fix:
1. The directionContext gate used `'inlineDirection' in directionContext`
which fires for keys with `undefined` values. The resolver can produce
`inlineDirection: undefined` when no paragraph w:bidi is set anywhere
in the cascade, and the function would then return false instead of
falling through to the legacy direction/dir field. Check the value, not
the key.
2. Anchor the legacy direction/dir fallback as compat-fallback per
comment-policy.md so future agents know what triggers it (no typed
directionContext) and when it can be retired (SD-2778 collapses the
duplicate field).
3. Document why `attrs.textDirection` is no longer in the chain. Per ECMA
§17.18.93, ST_TextDirection values are writing-mode (lrTb/tbRl/btLr/
lrTbV/tbRlV/tbLrV); none equal 'rtl'. The old check was always dead.
New test covers the precedence edge case.
---------
Co-authored-by: Caio Pizzol <caiopizzol@gmail.com>
…or (#3201) Extract resumePackagePublish switch into per-descriptor resumePublish functions and replace pkg.name === 'sdk' branches with capability checks (pkg.pythonPackages, pkg.preparePythonSnapshot). No behavior change: the recovery engine becomes generic so adding superdoc/react/vscode-ext in follow-up PRs only adds adapters, not new switch arms. The internal field state.sdkPythonPublished is renamed to state.pythonPublished and recovery's returned snapshot field sdkPythonSnapshot to pythonSnapshot. recordSdkPythonSnapshot keeps its name so it continues emitting the sdk_python_snapshot_* GITHUB_OUTPUT keys consumed by release-stable.yml. Existing helper tests updated to match the refactored structure (the intent - each package has its own explicit resume path - is preserved).
#3120) * chore: save * feat: update TOC entry in context menu * fix: update TOC creating extra spaces and changing fonts * test: created tests around TOC programmatic update * refactor: removed unused code * refactor: code style tweaks * refactor: removed duplicate functions * test(toc): add regression coverage for SD-2664 review findings - tabLeader: 'none' must round-trip via serialize/parse (currently lost because no \\p is emitted when separator is missing, and the parser has no way to disambiguate "default = dots" from "explicit none"). - toc.configure({ tabLeader: 'none' }) on a default-leader TOC must not silently no-op (areTocConfigsEqual reports identical serialized output). - toc.update mode: 'pageNumbers' must find tocPageNumber marks when the marked text is nested inside a run wrapper (the rebuild output shape). All three tests fail on the current branch and lock in the regressions flagged in code review. * fix: toc context menu update * refactor: simplified logic * refactor: removed unnecessary test suite * refactor: simplified tests * chore: small comment tweaks * test: added behavior test for multiple TOCs updates * fix: inline partial selection to produce inline text * fix: toc from empty to non-empty * fix: early return on TOC update --------- Co-authored-by: Gabriel Chittolina <gabrielchittolina1@gmail.com> Co-authored-by: Caio Pizzol <caio@superdoc.dev>
… paragraphs (SD-2973) (#3210) * fix(converter): preserve hyperlink mark on inserted text split across paragraphs (SD-2973) SD-2858's preserveRaw path keeps tracked-change-wrapped fields structurally intact when the field crosses paragraph or wrapper boundaries, avoiding the import crash. For destructive wrappers (w:del / w:moveFrom) this trade-off is invisible since the content disappears on accept, but for constructive wrappers (w:ins / w:moveTo) the user keeps the inserted text and Word shows it both as inserted AND as a clickable hyperlink — the previous behaviour rendered it with insertion styling alone. Add an `isConstructiveTrackChangeElement` predicate, propagate a `preserveRawConstructive` flag through the field collector, and run a post-pass that finds the visible runs (between separate and end fldChars) and wraps them in `w:hyperlink` in-place. The surrounding paragraph and tracked-change wrapper structure is left intact so the SD-2858 round-trip guarantee still holds. * refactor(converter): share hyperlink attribute resolution between field paths (SD-2973) Per Luccas's review on PR #3210: the URL/anchor parsing and rels-element construction in applyConstructiveFieldInterpretation duplicated the same logic in preProcessHyperlinkInstruction. Extract resolveHyperlinkAttributes(instruction, docx) as the single source of truth for parsing a HYPERLINK field instruction into the attribute set that belongs on a w:hyperlink element. Both preProcessHyperlinkInstruction (the standard field path) and applyConstructiveFieldInterpretation (the SD-2973 raw-preserved constructive-tracked-change path) call it. Net change: ~-5 lines, single source of truth, no behaviour change.
* fix: footer tcs in replacement generating one per character (#2965) * fix: footer tcs in replacement generating one per character * fix: sdt with "contentLocked" not removable * chore: removed console log * fix: allow single click to target whole field --------- Co-authored-by: Nick Bernal <117235294+harbournick@users.noreply.github.com> Co-authored-by: Gabriel Chittolina <gabrielchittolina1@gmail.com>
…#3212) Adds two color swatches to the toolbar that re-theme tracked changes and comment highlights by writing the public --sd-* CSS variables on :root, and centers the editor in its column with a max-width wrapper. - Native <input type="color"> hidden behind a tb-btn label keeps the no-UI-kit posture of the demo. - Icons inside each swatch use mix-blend-mode: difference so they stay legible across any picked color. - The editor sits inside an .editor-canvas wrapper (max-width 880px, margin auto) so the document is centered between the toolbar and the activity sidebar.
* feat(workflows): notify homepage repo on stable superdoc release Mirrors the promote-stable-docs.yml pattern: workflow_run on Release superdoc, gated on success + stable, with a tag-diff against the triggering head_sha to filter out semantic-release no-ops. Sends repository_dispatch to superdoc-dev/homepage so a receiver workflow there can open one bump PR per release. Kept out of release-superdoc.yml on purpose: that job sits in the release-stable concurrency group, and a homepage/token failure should not mark a successful npm publish as failed. * refactor(workflows): simplify notify workflow to a single step
* fix: clear transient hyperlink styleId on unlink * test: add unlink regression coverage for transient hyperlink style cleanup * fix(link): derive underline preservation at unlink time and add imported-link regression * fix(link): tag paste-added underline as autoAdded so unsetLink removes it When a user pastes a bare URL, handlePlainTextUrlPaste auto-converts it to a link and adds an underline mark. The PR's autoAdded mechanism in unsetLink only removes underline marks tagged autoAdded:true, so the paste-added underline (untagged) was left behind on Remove Link. Same one-line fix as setLink at link.js:247. Adds regression coverage to relationships.test.js for: paste-URL unlink, setLink with longer replacement text, mixed-underline selections, and re-setLink. * test(link): drop paraphrase comments to match local convention Existing tests in relationships.test.js use no inline comments; removing the four added in the previous commit so the new tests match local style and the comment policy. --------- Co-authored-by: Caio Pizzol <caio@harbourshare.com> Co-authored-by: Caio Pizzol <97641911+caio-pizzol@users.noreply.github.com>
…ands (SD-3083) (#3217) * fix(super-editor): guard cached paragraph props lookup in indent commands (SD-3083) Increase/Decrease Indent crashed when fired before the rendering pass had populated the resolved-paragraph-properties cache (fresh load, freshly inserted paragraphs). Falls back to compute-on-miss so the style cascade is honored, instead of a bare guard which would stop the crash but apply the wrong delta on style-derived indents. Set/Unset commands keep the original code path - they don't read the current indent and don't need the resolve work. * test(super-editor): expand textIndent regression coverage (SD-3083) Unit tests: - decreaseTextIndent honors style-derived indent on cache miss (symmetric to the existing increase regression test) - Cache hit short-circuits the compute-on-miss fallback - inverse of the set/unset opt-out test, guards the production || short-circuit Behavior test (tests/behavior/tests/toolbar/paragraph-indent.spec.ts): - Increase Indent on a fresh paragraph adds indent without crashing - Decrease Indent removes the indent applied by Increase - Repeated Increase compounds the left indent * docs(super-editor): fix unsetTextIndentation @example typo Pre-existing typo - example used `unsetTextIndent()` but the function is `unsetTextIndentation`. Found during a comment audit on this branch.
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
* feat(super-editor,painter): render images inside Word textboxes (SD-2804) ECMA-376 §20.4.2.38 (CT_TxbxContent) lets a textbox hold rich body-level content — paragraphs whose runs can carry inline w:drawing images. The text-only extractor used to silently skip those drawings, so the textbox rendered empty even though export round-tripped the image untouched. The fix surfaces the inline drawing as a textContent part with kind='image' so the existing shape painter can render it alongside text spans: - TextPart contract gains optional kind/src/width/height/alt fields. - extractTextFromTextBox.handleRun branches on w:drawing, reuses the v3 wp drawing handler (handleImageNode) to resolve rId, then upgrades the path-style src to a data URI from converter.media so the painter can drop it straight into <img>. - DomPainter's createFallbackTextElement renders image parts as inline <img> elements next to existing text spans. Linked: SD-2745 (header-anchored floating textboxes — positions the box where this content now renders). * fix(super-editor,pm-adapter,painter): address PR #3207 review (SD-2804) Per Luccas's review on PR #3207: - (C1) Skip hidden textbox images. handleImageNode flags wp:docPr hidden="1" via attrs.hidden, but the new image-part branch only checked attrs.src and emitted visible <img>s for them. Top-level hidden drawings are filtered later in the pipeline; image parts bypass that filtering. Gate the textParts.push on imagePm.attrs.hidden !== true so hidden textbox drawings stay hidden, matching the body-level behaviour. - (C2) Drop the duplicated resolveImagePartSrc helper in the importer (it rejected Uint8Array, breaking Y.js binary media). Store the raw path + extension + rId on the image part. pm-adapter's hydrateImageBlocks gains a vectorShape branch that hydrates textContent.parts alongside ImageRuns, so all media path candidates and the Uint8Array → TextDecoder decoding live in a single place. - (C3) Anchored drawings inside textboxes are out of scope — wrap / position / transform metadata isn't carried into the text-parts model. Restrict the textbox-image branch to wp:inline and document the limit in the code comment so a future fixture can extend it intentionally. - (C4) Align inserted images to the text baseline like body inline images do (vertical-align: bottom). ECMA-376 §20.4.2.8 specifies that an inline drawing behaves "like a character glyph of similar size", and the body inline image renderer defaults to vertical-align: bottom (renderer.ts ~L5770, L5847) — the textbox image part used vertical-align: middle, visibly misaligning text next to the image inside a textbox compared to outside it.
…stop) (#3204) Adds superdoc to the stable orchestrator so the v* tag drives docs-stable promotion in the same workflow that releases tools, removing the cross- workflow concurrency-eviction problem for stable superdoc releases. The orchestrator now groups packages by chain. Within a chain, fail-stop applies as before (CLI failure skips SDK/MCP). Across chains, failures are independent: a tools failure does not skip superdoc and vice versa. Workflow rewiring: - release-superdoc.yml stops auto-firing on stable; main pushes still publish prereleases, and workflow_dispatch is preserved for recovery. - promote-stable-docs.yml triggers off release-stable.yml. The conclusion gate now accepts both success and failure - a tools-chain failure that follows a successful superdoc release should still promote docs. The inner git-tag detection (compare tags merged at the run's head_sha vs origin/stable) remains the source of truth, so tools-only runs still leave docs-stable alone. - release-stable.yml header comments + step name updated to reflect the broader scope; the workflow's name field is unchanged so the existing workflow_run trigger and concurrency group continue to match. A rename is best as a follow-up cleanup PR. Stacked on #3201 (descriptor refactor).
Adds react to the core chain after superdoc. react consumes `superdoc` in dependencies, so releasing them in order through the orchestrator means consumers never see a react release that pins to an older superdoc than what just shipped. - Adds `resumeReactPublish` adapter (uses generic `npm-publish-package.cjs` helper, idempotent against npm via dist-tag updates). - Adds react descriptor to the `core` chain. A superdoc failure now skips react (fail-closed within chain); a tools failure still does not. - `release-react.yml` stops auto-firing on stable; main pushes still publish prereleases. Stacked on #3204 (superdoc orchestrator).
Completes the core chain: superdoc -> react -> vscode-ext. vscode-ext publishes to the VS Code Marketplace (not npm) and ships a .vsix asset on the GitHub release; the script's existing helpers already cover both, so this PR adds the descriptor, the resume adapter, and the workflow plumbing. - `resumeVscodeExtPublish` runs `pnpm run package` then `vsce publish --skip-duplicate`; idempotent against the marketplace. In a tagged snapshot it also runs `build:superdoc` first so esbuild can resolve the webview's `superdoc` and `superdoc/style.css` imports through packages/superdoc/dist (snapshot only ran `pnpm install`). - vscode-ext descriptor uses `vsCodeExtensionId` (no `npmPackages`), so `inspectPackageReleaseState` probes the marketplace, not npm. - release-stable.yml's orchestrator step gains `VSCE_PAT` env so vsce can authenticate. - release-vscode-ext.yml stops auto-firing on stable; main pushes still build .vsix attachments to the GitHub release. - Three remaining `pkg.name === 'vscode-ext'` branches in the script (`getExpectedReleaseAssets`, `isGitHubReleaseComplete`, `ensureGitHubRelease`) switched to `pkg.vsCodeExtensionId` capability checks for consistency with PR #3201's refactor pattern.
Stable superdoc releases now ship via release-stable.yml (named '📦 Release stable tooling (CLI/SDK/MCP)'), so listening to '📦 Release superdoc' on workflow_run silently stopped firing for stable releases. Mirrors the trigger change already in place on promote-stable-docs.yml. - Trigger now matches the orchestrator workflow name. - Accept conclusion: failure too, so a tools-chain failure following a successful superdoc release still notifies (chain-independent orchestrator semantics). - Verify both 'superdoc' and '@harbour-enterprises/superdoc' on npm before dispatching; semantic-release's prepare phase pushes the v* tag before publish, so a publish failure that does not recover would otherwise notify on an unpublished version.
buildTocEntryParagraphs wraps the page-number text inside a run, so the tocPageNumber mark lives one level below the paragraph's direct children. sanitizeTocContentForSchema only filtered marks on direct children, so any schema that omits tocPageNumber (the case the sanitizer exists to support) would still receive the unknown nested mark and nodeFromJSON would fail when rebuilding the TOC. Extracts the per-node mark-stripping into a recursive helper that walks content arrays, so the sanitizer works regardless of how deep buildTocEntryParagraphs nests the run wrapper. Verified empirically before fixing: - Reproduced the bug against a real prosemirror-model Schema lacking the tocPageNumber mark: unfixed sanitizer leaves the nested mark and nodeFromJSON throws 'There is no mark type tocPageNumber in this schema'; fixed sanitizer strips it and nodeFromJSON succeeds. - Codified as 4 unit tests in toc-wrappers.test.ts (sanitizer exported for testing). With the fix reverted, 2 tests fail with the exact surviving-mark assertion. With the fix applied, all 13 tests pass. Flagged by chatgpt-codex-connector in #3228.
* fix: missing headers in collab mode; wrong odd/even headers * fix: rebuild cache and clean up * fix: improve odd/even header selection
…el (SD-3109) (#3243) The Document API write side accepts text offsets in a flattened model: text contributes its length, leaf atoms contribute 1, inline wrappers (run, etc.) contribute 0. Position readback was using raw PM arithmetic (`pos - resolved.start(depth)`), which counts the wrapper boundary tokens the flattened model skips. Result: `editor.doc.bookmarks.list()` returned offsets that drifted by the number of inline wrappers in the targeted block. - Route both readback helpers through `pmPositionToTextOffset` so read and write share one offset model. - Same fix applied in `permission-ranges-adapter.ts`, which carried a parallel copy of the buggy helper. - Regression test for the round-trip invariant (write at offset N, list returns offset N). - Includes regression coverage for SD-3108 (content-control wrap by text offset). That bug did not reproduce on main; the tests stay as guardrails.
…(SD-2781) (#3203) * feat(pm-adapter): preserve run-level bidi/script metadata on TextRun (SD-2781) Adds two preservation-only fields to the layout text-run contract, kept on separate axes per ECMA's own categorization: - TextRun.bidi (RunBidiContext): direction signals only - run rtl flag now, embedding (w:dir) and override (w:bdo) wired in Wave 1c. - TextRun.script (RunScriptContext): complex-script flag + per-script language metadata (default / complexScript / eastAsian) per §17.3.2.20. Both populated by pm-adapter from raw run properties when present. Wave 1a does not render either; Wave 1b will gate the formatting-stack selection on script.complexScript, Wave 1c will read bidi.embedding/override. Why now: ECMA puts direction (rtl, dir, bdo) and script formatting (cs, lang/@bidi) in different categories. Lumping them under one bidi field would collapse the axes and lie about the schema. Adding both fields now means Wave 1b/1c don't have to introduce both the data path and the rendering at once. The RunScriptContext.language field expanded from a single string to a structured object with three optional tags. No production consumer reads the field yet (preservation-only since #3184), so the shape change is safe. Tests: - bidi populated only on rtl, script populated only on cs/lang - explicit rtl=false preserved (a meaningful override) - the three lang tags land on separate fields - axis non-collapse: rtl never leaks into script, cs never leaks into bidi * fix(contracts): import RunBidiContext + RunScriptContext locally for TextRun The previous commit added `bidi?: RunBidiContext` and `script?: RunScriptContext` to TextRun in contracts/src/index.ts but only re-exported the type names; they were not in local type scope, so `tsc --project` failed: src/index.ts(324,10): error TS2304: Cannot find name 'RunBidiContext'. src/index.ts(330,12): error TS2304: Cannot find name 'RunScriptContext'. Vitest's transform doesn't enforce type-only imports the way tsc does, so the unit tests passed even though the build was broken. CI build would have caught it; the missing piece was running `pnpm build` locally before pushing. Adds the two names to the existing local `import type` statement alongside ParagraphDirectionContext (same pattern, same line 19). Also adds two integration tests in pm-adapter/src/integration.test.ts that exercise the full PM -> FlowBlock conversion through the unmocked applyInlineRunProperties pipeline. The previous unit tests in common.test.ts mock computeRunAttrs, which is why they couldn't catch shape-level regressions in the contracts package. The integration tests prove a real PM doc with raw runProperties (rtl/cs/lang on a run-wrapper node) produces a TextRun with populated bidi/script, and that runs without signals don't gain empty objects. * docs: drop duplicate ticket prefix on SD-2781 test block comment Repo convention puts the ticket reference in the `describe` label only (e.g., `describe('SD-1333: ...')`). Trims the redundant `SD-2781:` prefix from the block comment, keeps the non-obvious "Wave 1a preserves these signals; nothing renders them yet" note. * fix(pm-adapter): read bidi/script from raw inline runProperties + reassign on token runs Two codex findings on PR #3203: 1. Cascade-leak: applyInlineRunProperties was receiving cascade-resolved runProperties from runNodeChildrenToRuns and populating bidi/script from them. Style-inherited runs ended up with bidi/script metadata they did not have inline, making preservation indistinguishable from direct formatting and bloating the layout tree on every styled run. Fix: thread the raw inline runProperties through InlineConverterParams (alongside the existing cascade-resolved runProperties). applyInlineRunProperties gains a fourth parameter that bidi/script populate from. When the caller doesn't opt in (no inline parameter), no metadata is attached - safer default than reading the cascaded view. 2. Token-run drop: generic-token.ts called applyInlineRunProperties without reassigning the return value. Since the helper builds a new object via spread, all merged fields (including the SD-2781 bidi/script) were lost for page-number / total-page-count token runs inside an rtl run wrapper. Fix: change the local to `let` and reassign. Also forwards inlineRunProperties through. no-break-hyphen.ts (another caller) updated for consistency. Tests added: - 3 unit tests in common.test.ts proving bidi/script populate from inlineRunProperties only, never from cascade-resolved runProperties - Existing 7 SD-2781 unit tests updated to pass inlineRunProperties (the new opt-in source) - 1 integration test proving bidi/script propagate to page-number tokens inside an rtl run wrapper All 1800 pm-adapter tests, 12644 super-editor tests, and 1201 layout-bridge tests pass. Contracts + pm-adapter both build clean. * fix(pm-adapter): cs absence != false + forward inlineRunProperties through nested converters Two round-3 findings on PR #3203: 1. RunScriptContext.complexScript was required (boolean), so a run with only w:lang and no w:cs ended up with { complexScript: false, language: {...} }. Per ECMA §17.3.2.7, absent w:cs inherits from the style hierarchy and ultimately falls back to Unicode-based script detection - it is NOT the same as explicit false. Misrepresenting absence as false would mislead Wave 1b CS-formatting selection. Fix: make complexScript optional. Only set it when raw cs is explicitly present (true OR false - both are meaningful toggle states per §17.17.4). Leave undefined otherwise so consumers can fall back correctly. 2. Three nested inline converters (page-reference, bookmark-start, structured-content) called visitNode without forwarding the new activeInlineRunProperties arg, so text inside a PAGEREF result, a bookmark span, or an SDT wrapper lost run-level bidi/script metadata even when the enclosing run wrapper had it. Fix: thread inlineRunProperties through all three converters. In page-reference's bookmark path, also pass the locally-scanned run's raw runProperties (not the cascade-resolved version) as inlineRunProperties for the synthesized token run. Tests: - cs absent + only lang -> script exists, complexScript is undefined - cs=false explicit -> script.complexScript === false (meaningful off) - existing 'three lang tags' test unchanged - new integration test: text inside structuredContent wrapper preserves bidi/script from the enclosing run wrapper All 1803 pm-adapter tests pass (+3 new), 12644 super-editor pass, contracts + pm-adapter both build clean. * docs: drop round-counter prefix from SD-2781 integration test header Per project CLAUDE.md, code comments shouldn't reference iteration / review state ("round-3 (codex finding)") - those belong in the PR description and rot once the PR merges. The load-bearing content (which converters, what invariant) stays. --------- Co-authored-by: Caio Pizzol <caiopizzol@gmail.com>
* feat(types): deep public-type audit gate (SD-2977) The existing public-type checks lock in exported type names and top-level `any` assertions, but they do not catch `any` on public members, callback params, return types, or nested type arguments. Consumers can still lose IntelliSense after touching a public API surface that resolves to `any`. Add a deep public-type audit that builds a TypeScript Program from the packed-and-installed `superdoc` tarball, recursively walks every owned type reachable from public export entries, and fails CI if a finding is not in `deep-type-audit.allowlist.json`. Also fails on stale allowlist entries (so fixes must remove their entry), on compiler diagnostics in the published surface, and on private `@superdoc/*` specifiers that survived rewriting in the installed package. Allowlist seeded with the current 293 owned findings, tagged by tier so follow-up PRs can drain by surface (Pinia stores, toolbar config, helpers, curated public contract, other). The gate runs in PR CI and release CI, after the consumer matrix prepares the installed tarball. Closes the gap where release publish did not run packed consumer checks. The allowlist is a starting line, not a waiver. Per the README: do not drain by replacing `any` with `unknown` unless the value is genuinely opaque - prefer precise upstream or local types so IntelliSense actually restores. Verified locally: - node tests/consumer-typecheck/typecheck-matrix.mjs: 53 passed - node tests/consumer-typecheck/deep-type-audit.mjs: 293 findings, 0 new, 0 stale - removing an allowlist entry produces NEW finding output and exit 1 * fix(types): walk construct signatures and index types in deep audit (SD-2977) Codex review on the initial PR pointed out two walker blind spots: 1. The walker only visited `getCallSignatures()`, so a public `constructor(...args: any[])` (SuperConverter, DocxZipper, and others) shipped without producing a finding. 2. The walker only enumerated `getProperties()`, so `[key: string]: any` index signatures on public classes were never traversed. Both gaps confirmed in the installed dist before the fix. Walker now visits construct signatures alongside call signatures and queries `getStringIndexType()` / `getNumberIndexType()` for index members. New findings are tagged `tier-4-public-contract` for SuperConverter and DocxZipper (per SD-2966's done-when criteria) and `tier-5-other` for the remaining constructor-surface findings. Allowlist regenerated: 293 → 337 entries. Verified: - node tests/consumer-typecheck/deep-type-audit.mjs: 337/0/0 PASS - SuperConverter[string] and DocxZipper[string] now in tier-4 * fix(types): close 3 deep-audit walker blind spots (SD-2977) Code review surfaced three coverage gaps in the seeded allowlist: 1. Sibling members sharing an instantiated container type (`a: any[]; b: any[]`) were path-order-dependent. TypeScript caches `Array<any>` and `Promise<any>` per-shape so siblings share a type id; the persistent-visited gate short-circuited the second sibling before its inner `any` could be recorded. Fix: pre-record `any` inside array elements and type arguments BEFORE the visited gate. Cycle protection still applies to structural members. 2. Class exports walked only the instance side (getDeclaredTypeOfSymbol), so `constructor(...args: any[])` and `static foo(): any` on public classes (SuperConverter, DocxZipper) never produced a finding. Fix: walkExport now also walks the value type (getTypeOfSymbolAtLocation) when it differs from the declared type, prefixed with `.<value>` so constructor / static findings are clearly distinguished. 3. `--pack` bootstrap failed on fresh checkouts because typescript was resolved at module-load (line 49) before the doPack block (line 52) that runs the fixture install. Fix: move the typescript require after the optional pack-and-install block so the documented bootstrap command actually works on a clean tree. A stack-scoped visited variant was tried first but caused combinatorial blow-up on highly interconnected types (16+ minutes with no progress on the public surface). The pre-record + persistent-visited combination keeps cycle protection bounded while still catching sibling regressions. Allowlist regenerated: 337 -> 678 entries, with new tier-4 entries covering SuperConverter / DocxZipper class-side `any`s and tier-5 absorbing the long tail of sibling-shared findings. Verified: - node tests/consumer-typecheck/deep-type-audit.mjs: 678/0/0 PASS - SuperConverter.<value>.new(args)<0>, .new(args)[], static methods all in tier-4 - DocxZipper.<value>.new(args)<0>, .new(args)[] all in tier-4 - Run completes well under 60 seconds * fix(types): raise deep-audit depth cap and surface truncation (SD-2977) A third review round flagged three potential walker issues. Two were stale (P1: SuperConverter/DocxZipper class-side findings — already caught in fcf84a2, 16 entries in allowlist; P2: visited-singleton-any skip — already prevented by isAnyType returning before the visited gate in walkType lines 220-223). The third is real: MAX_DEPTH=8 silently truncated 302,642 paths in a single audit run, leaving deep public types invisible to the gate and contradicting the README's claim of walking every reachable type. Persistent visited handles cycles, but TypeScript materializes generic instantiations on demand with fresh type ids that visited cannot dedupe, so the cap is load-bearing memory protection (cap=256 OOM'd at ~4GB). Empirical sweet spot at MAX_DEPTH=16: deep enough to reach 451 more legitimate findings (allowlist 678 -> 1129), shallow enough to bound memory. depthCapHits counter now surfaces in the run report so any remaining truncation is visible rather than silent. The new findings are concentrated in pinia store internals, vue reactive chains, and prosemirror type expansions — surface areas the team already knows are noisy. The cap warning lets future maintainers decide whether further bumps are worth the memory cost. Verified: - node tests/consumer-typecheck/deep-type-audit.mjs: 1129/0/0 PASS - WARN line surfaces depth-cap hits in the report - No OOM at cap=16 * fix(types): cleanup audit per code review (SD-2977) Three small cleanups bundled: 1. Strip em dashes from README and code comments. User CLAUDE.md prohibits em dashes in all output including code comments and docs; replaced with ":", "(", or "." per surrounding grammar. 2. Drop "flagged by Codex on the initial PR, fixed here" trailing clauses on two comments. The surrounding rationale (call sigs vs construct sigs; index sigs vs properties) is load-bearing and stays; only the review-process narration goes. CLAUDE.md says comments should not reference the current task or fix. 3. Fix walkExport's stale "stack-scoped" comment + add visited snapshot/restore around the value-side walk. The previous comment claimed visited pops on exit (the failed try/finally variant), but the actual code uses persistent per-root visited. That meant structural types reachable from both an export's instance and value sides were silently skipped on the value side. Snapshot/restore scopes the value walk's visited freshly without polluting subsequent exports' declared walks. The visited fix surfaced 670 previously-hidden findings (1129 -> 1799), 990 of which live on .<value> paths (class value side). Composition reinforces the umbrella framing: most additional findings are Pinia store internals and other surfaces that SD-2966's facade should hide, not type. Verified: - node tests/consumer-typecheck/deep-type-audit.mjs: 1799/0/0 PASS - 0 em dashes remaining in either file - 0 "flagged by Codex" comments remaining Two related walker gaps (sig.thisParameter and `<T = any>` defaults) were investigated and confirmed real but have zero current matches in the published superdoc dist; deferred as future-hardening rather than landed pre-emptively. * refactor(types): pivot audit to report-only inventory; defer gate to post-SD-2966 (SD-2977) Three independent code-review opinions converged on the same conclusion: landing a 17K-line allowlist of accidental public surface is the wrong move. The current 1799 findings are largely from Pinia stores, EventEmitter generics, Vue SFC component types, and other code that was never deliberately committed as public API. Committing them as an allowlist would risk legitimizing internals as the type contract and forcing the team to type things that should be hidden. Pivot: 1. Delete the 17K-line `deep-type-audit.allowlist.json`. The walker stays; the artifact does not. 2. Make the audit report-only by default. Always exits 0 unless the script itself errors. Prints inventory: total findings, by-tier counts, top files, depth-cap warnings. 3. Add `--strict` flag for the eventual gate behavior. Not used in CI yet because it's only meaningful once SD-2966 defines the public facade and the allowlist is re-seeded against that smaller surface. 4. CI workflows updated: both PR CI and release CI run the audit in report-only mode. Comments explain that `--strict` is added later. 5. README rewritten with prominent "Status: report-only inventory" at the top, explaining what defers the gate and what re-emerges after SD-2966. What this delivers today: - The walker logic ships and runs in CI from day one - The 10K-line public artifact goes away - Inventory data appears in every CI run (visible signal of how much accidental surface is leaking) - No commitment to typing 1799 entries on accidental surface What this defers: - Hard gate against new regressions on the current accidental surface. Net cost: a few weeks where new `any` could land on already-`any`-heavy code. Acceptable because the work to drain that surface shouldn't start before SD-2966 anyway. Verified: - node tests/consumer-typecheck/deep-type-audit.mjs: PASS, exit 0 - node tests/consumer-typecheck/deep-type-audit.mjs --strict: FAIL, exit 1 - node tests/consumer-typecheck/deep-type-audit.mjs --write: still works (creates allowlist; intended for use post-SD-2966) * refactor(types): make audit report-only; add --strict for post-SD-2966 gate (SD-2977) Companion to the allowlist removal: rewires the walker so default mode prints inventory and always exits 0, while a new --strict flag preserves the original gate behavior for use once SD-2966 defines the public facade. - Compiler diagnostics, private specifier leaks, and allowlist comparison all switch from process.exit(1) to print-and-continue unless --strict is set - New report section prints by-tier and top-files breakdowns regardless of mode (visible CI signal) - When no allowlist file exists, the run prints an explanatory note pointing at SD-2966 and the --write workflow - CI step comments updated in both ci-superdoc.yml and release-superdoc.yml to reflect the report-only intent - README rewritten with prominent "Status: report-only inventory" section at the top; commands section documents --strict and clarifies --write is intended for post-SD-2966 baselining Verified: default exits 0, --strict exits 1, --write still seeds an allowlist correctly.
#3199) * feat(pm-adapter): plumb body section direction context through resolver Per ECMA §17.3.1.41, paragraph w:textDirection inherits from the parent section when omitted. The previous call site built sectionContext from `undefined`, so directionContext.writingMode was always 'horizontal-tb' even when the body's w:sectPr declared a vertical writing-mode. This wires SectionDirectionContext through ConverterContext, populated once at top-level conversion from the body sectPr. Paragraphs that omit their own w:textDirection now correctly inherit writing-mode. Scope: - Body-level sectPr only. Per-paragraph-section variation (each section with its own sectPr) and table-cell direction context are not yet plumbed through. Both gaps are documented inline and tracked under SD-2777 (migrate remaining direction-aware consumers). - No consumer currently reads directionContext.writingMode in production, so this fixes the data contract before the first consumer arrives. Tests: - New: paragraph inherits body sectionDirectionContext.writingMode - New: paragraph w:textDirection still wins as explicit override * fix(pm-adapter): recompute sectionDirectionContext per-document Codex finding on PR #3199: when callers reuse one ConverterContext across documents (toFlowBlocksMap does this), the previous `??` cache let the first document's body sectPr resolve once and stick. A vertical doc 1 followed by a horizontal doc 2 would have doc 2's paragraphs inherit doc 1's writing-mode. Fix: drop the `??` and always overwrite. The shared ConverterContext is mutated freshly each call before children read it, so per-document recomputation is enough. (The pre-existing `sectionDirection` field on the line above has the same pattern but is out of scope for this PR.) Test added: toFlowBlocksMap with two docs (vertical w:textDirection then horizontal w:textDirection) sharing one converterContext - asserts each doc's paragraphs get their own writingMode. Failed before the fix because the cached vertical-rl persisted into the horizontal doc; passes after. --------- Co-authored-by: Caio Pizzol <caiopizzol@gmail.com>
* fix(mcp): rename wire fields to contract names before dispatch The MCP wire schema (generated from apps/cli operation params) exposes PARAM_FLAG_OVERRIDES renames such as `commentId` → `id` and `parentCommentId` → `parentId`. Without an inverse translation at dispatch time, the contract validator rejects the wire field names because it expects the canonical contract names — so `superdoc_comment` actions `delete`/`get`/`update` fail with `comments.<action> commentId must be a non-empty string` even when the caller follows the documented schema. The CLI applies the inverse rename in extractInvokeInput() (apps/cli/src/lib/invoke-input.ts PARAM_RENAMES). This commit adds the matching layer for the MCP path: a small `applyParamRenames` helper in a standalone module, applied in `executeOperation` immediately before `api.invoke`. Affected operations: comments.create (parentId → parentCommentId), comments.patch / comments.delete / comments.get (id → commentId), getNodeById (id → nodeId). Includes unit tests for the rename helper. * review: fix comment count/style and document output-rename asymmetry - param-renames.ts: fix "Three" → "Five" (5 operations, not 3) and replace em-dash per style rule (caio-pizzol review suggestion). - intent.ts: add AIDEV-NOTE explaining why input renames are not inverted on output — canonical names in responses are usable directly, so inverting adds complexity for no practical benefit. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * review: clarify output-rename asymmetry is incidental, not deliberate Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Caio Pizzol <97641911+caio-pizzol@users.noreply.github.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
* fix: align rtl date token rendering with word parity Fix RTL date rendering parity with Word per SD-3098. The browser's UBA does not reorder numerics inside an RTL run the way Word does, and run-boundary separator drift breaks mixed-direction date strings like `-03-23` + `2026`. The fix is paint-time only - it never touches PM/model/export: 1. DomPainter sets dir="rtl" on the span when run.bidi.rtl === true (per-run bidi isolation eliminates run-boundary separator drift) 2. DomPainter sets dir="ltr" on date-like LTR runs (per regex) inside RTL contexts (prevents the third case where a non-rtl date inside an rtl paragraph reorders unpredictably) 3. normalizeRtlDateTokenForWordParity injects U+200F (RLM) around `./- ` separators inside RTL date-like text so Word and SuperDoc render the same visual order (e.g. XML `23/03/2026` -> visual `2026/03/23`) Three test cases in rtl-dates.docx (the Linear-attached "Date Being Weird" fixture): - Header: single <w:rtl/> run `23/03/2026` -> Word visual `2026/03/23` - Body 1: LTR run `-03-23` + RTL run `2026` -> Word visual `2026-03-23` - Body 2 (control): single LTR run `2026/03/26` -> unchanged Other changes: - bidiCompatible guard in mergeAdjacentRuns: prevents a <w:rtl/> run from merging with a plain run and silently losing the bidi flag - run-visual-marks.ts and versionSignature.ts: include run.bidi in the caching hashes so a rtl-only edit invalidates measure/DOM cache - New painter unit tests (rtl-date-parity.test.ts) verifying dir + RLM - New behavior spec (rtl-dates-word-parity.spec.ts) using the real fixture This PR builds on the run-level bidi metadata SD-2781 (#3203) added to the TextRun contract - reads from TextRun.bidi.rtl (the merged shape), not a parallel RunMarks.bidiContext field. * test(rtl-date-parity): cover bidi hash + block-version + painter edge cases Adds pre-merge coverage for SD-3098 rendering invariants: - hashRunVisualMarks: bidi field changes the dirty-run hash (rtl=true vs absent, rtl=true vs rtl=false, embedding-only changes). Stale hashes would let an edit that flips just <w:rtl/> reuse stale measure/DOM. - deriveBlockVersion: bidi flips invalidate the cached block version. Without this, the painter would reuse a cached block snapshot after an rtl-only edit. - DomPainter painter tests: mixed rtl + ltr runs on the same line stay as separate spans with distinct dir attrs; non-date-like rtl runs keep dir="rtl" without RLM injection; non-rtl plain text leaves the span without a dir attribute. Also adds rtl-mixed-run-line.docx + behavior spec as a negative test asserting Hebrew + date + Hebrew paragraphs don't regress (Hebrew runs stay rtl, date run is not RLM-injected since it isn't rtl-tagged). --------- Co-authored-by: Caio Pizzol <caiopizzol@gmail.com> Co-authored-by: Caio Pizzol <97641911+caio-pizzol@users.noreply.github.com> Co-authored-by: Caio Pizzol <caio@superdoc.dev>
* feat(types): harden package shape (SD-2978)
* fix(types): gate stable release with same package-shape checks (SD-2978)
The original SD-2978 commit only added consumer matrix + package-shape
gates to release-superdoc.yml. Between sessions, release-stable.yml
landed as the new central orchestrator for the npm `latest` publish
lane, and SuperDoc's stable releases now route through that workflow
instead of release-superdoc.yml (which is now `@next` only).
Without this patch the stable lane would publish without the matrix or
publint/attw gates running, defeating the purpose of "package-shape
honest in CI" because the most-consumed dist-tag would still be
unverified at publish time.
Adds the same three steps (matrix, deep-type-audit, package-shape-gate)
between Build packages and the orchestrator step in release-stable.yml.
* fix(types): teach deep audit to read nested types: { import, require } (SD-2978)
The deep audit assumed `entry.types` is always a string. SD-2978's
manifest changes nest it as `{ import: '...d.ts', require: '...d.cts' }`
for the three entries that publish CJS. The audit threw
ERR_INVALID_ARG_TYPE on `path.resolve(root, entry.types)` when entry.types
was an object.
Add a small helper that picks the ESM target from either shape (string
or condition object). Walking the .d.ts side is sufficient because the
.d.cts is a generated shim of the same surface.
Verified: audit now exits 0 with the same 1799 findings as pre-fix.
* fix(types): make sanitizer re-entrant + route pack:local through pack:es (SD-2978)
Code review found that `pnpm run pack` was broken by the new
prepack/postpack lifecycle. pnpm wraps prepack/postpack around scripts
named exactly `pack`, and the user `pack` script itself invokes
`pnpm pack` which triggers a second prepack. The inner prepack hit the
"backup exists" guard and exited 1, the outer postpack was skipped, and
the workspace was left with `package.json` mutated and
`.package.json.prepack-backup` orphaned.
Two changes:
1. Make `prepare` re-entrant. If the backup file exists AND the current
manifest already looks sanitized (no `source` conditions, no `unpkg`
or `jsdelivr` fields), no-op so the inner prepack falls through and
the inner postpack can restore cleanly. If the backup exists but the
manifest is NOT sanitized, fail loudly with a clear message — that
means the workspace is in an inconsistent state from a previous
failed pack and the developer needs to clean up. `restore` was
already idempotent (no-op when backup missing).
2. Route `pack:local` through `pack:es` directly. Both ultimately do
the same thing, but going through `pack:es` (whose name does not
collide with the lifecycle trigger) avoids the double-fire on the
common local-pack path.
Verified with a synthetic harness covering 5 cases: clean run,
double-fire (outer + inner), failed run (state inspection), retry after
failure (self-heal), inconsistent state (loud refusal). All pass.
Verified live in the worktree:
- pnpm run pack:es: tarball created, manifest restored, no orphan backup
- pnpm run pack: same (was broken before this commit, now works)
…3214) * feat(converter): render EMF+ images via embedded bitmaps (SD-2503) EMF+ payloads use GDI+ drawing records that rtf.js doesn't implement, so prior to this change every EMF+ image rendered as the "Unable to render EMF+ image" placeholder. Most real-world EMF+ files generated by Office (cover slides, charts, illustrations) embed a complete PNG/JPEG inside an EmfPlusObject(Image) record with BitmapDataType=Compressed. Walk the EMR_COMMENT records in the EMF stream, parse the inner EMF+ records, reassemble continuation series, and return the embedded image directly. Per MS-EMFPLUS § 2.3.5.1 the EmfPlusObject header layout depends on the ContinueBit: ContinueBit=1: Type(2) Flags(2) Size(4) TotalObjectSize(4) DataSize(4) ObjectData ContinueBit=0: Type(2) Flags(2) Size(4) DataSize(4) ObjectData TotalObjectSize is present on every continued record (not only the first). The strict spec terminates a continued series with a ContinueBit=0 record; the parser also flushes early once TotalObjectSize bytes have been accumulated as a defense against off-spec encoders that leave ContinueBit=1 on the final record. Pure-vector and pixel-format EMF+ images still fall back to the placeholder — a full GDI+ rasterizer is out of scope here. Tests use synthetic in-memory EMF+ buffers and cover PNG/JPEG extraction, spec-compliant 2-record and 3-record continuation reassembly, off-spec early flush, the non-Image fallback path, and rejection of pixel-format bitmaps. Closes #3172 * feat(converter): render raw-pixel EmfPlusBitmap via canvas + review nits Address review feedback on #3214: 1. Raw-pixel EmfPlusBitmap support — the m3 proposal.docx reproducer from #3172 stores its image as raw pixels (BitmapDataType=Pixel), which the prior extractor rejected. Now decode 24bppRGB / 32bppRGB / 32bppARGB / 32bppPARGB pixel data, draw onto a canvas, and export as PNG (mirroring the tiff-converter pattern). Indexed formats and missing-canvas environments still fall back to the placeholder. EMF+ pixel formats store channels in DWORD-little-endian order (B,G,R[,A]); the converter swaps to canvas-native R,G,B,A. PARGB un-premultiplies alpha so straight-alpha consumers render correctly. Negative height = top-down rows; positive height = bottom-up (classic Windows DIB), reversed before write. A MAX_PIXEL_BITMAP_PIXELS guard bounds canvas allocation at 100M pixels (~400 MB RGBA), matching tiff-converter. 2. Slice reassembled chunks to TotalObjectSize so an off-spec writer that overshoots its declared size doesn't tack trailing bytes onto the data URI. 3. Tighten EMR_COMMENT recordSize check to >= 20 to match isEmfPlus's existing minimum. Tests: 6 new pixel-bitmap tests using a vi.spyOn canvas mock cover the core 32bppARGB path, bottom-up row flipping, 24bppRGB byte order, 32bppPARGB un-premultiplication, the no-canvas fallback, and the indexed-format fallback. 20/20 in this file, 300/300 across the helpers directory. * fix(converter): treat EMF+ raw pixels as top-down regardless of Height sign MS-EMFPLUS § 2.2.2.2 is silent on what Height/Stride sign means for storage direction. The earlier reading "positive Height = bottom-up" borrowed from the classic Windows DIB convention, but every GDI+ producer (which means every Office-generated EMF+) lays pixel memory out top-down regardless of Height sign. Rendering the SD-2503 reproducer with the bottom-up assumption produced an upside-down cover image. Drop the row-reversal entirely; storage row 0 is the visual top in all cases. Update the corresponding test and JSDoc to reflect the empirical convention. --------- Co-authored-by: Caio Pizzol <caio@superdoc.dev>
* fix: mirror explicit left/right paragraph alignment for rtl * test(rtl-paragraph-alignment): cover both/distribute + Word-fixture import path --------- Co-authored-by: Artem Nistuley <artem@superdoc.dev> Co-authored-by: Caio Pizzol <caio@superdoc.dev>
* fix(context-menu): fix slash menu dismissal state (SD-2747) The slash command menu had three independent state bugs that combined to break the dismiss-and-retype flow: 1. Backspace and Delete were not handled anywhere — neither the PM plugin's handleKeyDown nor the Vue component's document keydown listener caught them, so pressing Backspace after opening the menu left it open. 2. A 5-second slashCooldown locked out subsequent `/` presses immediately after dismissal. The user typed `/`, dismissed the menu, typed `/` again to retry — and got a literal `/` inserted instead of the menu reopening. 3. Escape closed the menu but did not insert the slash the user originally typed (it had been preventDefault'd on open). Per the requirements that match Google Docs, dismissing with Escape should leave the slash visible while dismissing with Backspace should remove it. Plugin handleKeyDown now handles Backspace / Delete (close, no insert) and Escape / ArrowLeft (close, insert `/` at the original anchor). The 5-second cooldown is gone — subsequent `/` reopens the menu immediately. Focus shifts to the Vue search input when the menu opens, so the PM plugin can't see keys typed there. The Vue handleGlobalKeyDown handler gets the same three branches (Backspace/Delete close without insert, Escape closes and inserts the slash) so the dismissal works whichever element holds focus. Removed the three unit tests that codified the cooldown behavior; added six new tests covering the corrected dismissal contract. * feat(context-menu): show search header + empty state for slash filter (SD-2747) While the menu is open, focus is on a hidden search input that captures keystrokes for filtering. The user saw no feedback — they typed `intex`, the filter eliminated all items, and the menu collapsed to a zero-height invisible box. Visually it looked like the menu had silently vanished. Two additions, scoped to the same menu: - A "Searching: /<typed text>" header appears at the top of the menu whenever the user has typed any filter characters. The header uses a monospaced font for the slash + query so it reads as "this is what you're literally typing," matching command-palette conventions. - A "No matching commands" empty state renders inside the items list when the filter has eliminated every item, so the menu always has visible content as long as it's open. Existing items, divider rendering, and selection state are unchanged. * fix(context-menu): gate slash reinsertion to slash-triggered opens (SD-2747) Addresses Codex's P1 and Luccas's review on PR #3234. Pressing Escape (or ArrowLeft) on a context menu opened via right-click previously inserted a literal `/` at the click position, mutating the document — the dismissal path assumed the menu was always opened by a suppressed `/` keystroke. Track which gesture opened the menu and gate slash-reinsertion on `trigger === 'slash'` everywhere. Also wires ArrowLeft into the Vue-side `handleGlobalKeyDown`: the menu's hidden search input owns focus while the menu is open, so the PM plugin's matching branch never fires in the live flow and ArrowLeft was simply swallowed instead of dismissing the menu. Plus three undeclared CSS variables flagged by Codex (`--sd-ui-menu-header-bg`, `--sd-ui-menu-text-muted`, `--sd-ui-font-mono`) now have :root defaults so consumers can theme them and the inline fallbacks in `ContextMenu.vue` are no longer the only source. Changes - `context-menu.js`: new `trigger: 'slash'|'rightClick'|null` field on the plugin state, set from the existing `isRightClick` signal in the 'open' meta and cleared on 'close'. The PM-side Escape/ArrowLeft branch only inserts `/` when `trigger === 'slash'`. - `ContextMenu.vue::handleGlobalKeyDown`: same `trigger === 'slash'` gate applied to the Vue-owned dismissal path. ArrowLeft now joins Escape in this branch. - `variables.css`: declare the three undeclared `--sd-ui-menu-*` tokens at :root with the same fallback values currently inlined in ContextMenu.vue. TDD - 5 new failing unit tests before the fix, 5 green after: - records `trigger='slash'` on keyboard open - records `trigger='rightClick'` on clientX/clientY open - clears trigger on close - Escape on right-click open: closes without inserting `/` - ArrowLeft on right-click open: closes without inserting `/` - All 64 context-menu plugin tests pass. - All 44 ContextMenu.vue component tests pass. Verification - Full `super-editor` suite: 12 716 / 12 716 pass. - Browser repro on the dev app: - Right-click → Escape: menu closes, doc unchanged (no stray `/`). - Right-click → ArrowLeft: menu closes, doc unchanged. - Slash → Escape: menu closes, `/` inserted at anchor (regression preserved). - Slash → ArrowLeft: menu closes, `/` inserted at anchor (was previously swallowed because the PM plugin branch never reached when hidden input holds focus).
…2911) (#3231) * fix(converter): preserve numbering.xml definitions on round-trip (SD-2911) #exportNumberingFile filtered out every w:num whose numId wasn't referenced from the exported document parts (including headers, footers, and footnotes). Word's tentative numbering, where a document carries definitions for lists the user hasn't applied yet, was therefore silently wiped: the active-numbering fixture lost 7 of 8 definitions and the tentative fixture lost them all. The filter was introduced for the lists.delete document-api operation, but it couldn't distinguish "user just deleted a list in this session" from "definition was always unused in the source file" — both arrived at the export with no referencing paragraph and both were dropped. Word tolerates unused definitions, so the safe default on export is to emit every abstractNum and num the importer captured. The strip-orphaned helper is removed entirely; the inline write in #exportNumberingFile is now four lines. Verified: both fixtures round-trip byte-identical (after pretty-print) at the numbering.xml level. New integration test covers both the active and tentative variants. * fix(converter): skip numbering.xml when source had none and body has no refs (SD-2911) The previous SD-2911 fix wrote `this.numbering`'s contents on every export. The importer falls back to `baseNumbering` when the source had no `word/numbering.xml`, so plain documents gained unused fallback definitions on round-trip — the regression Luccas flagged. Capture whether the source package shipped the numbering part before the baseNumbering fallback runs, and short-circuit the write when both: source had none AND no `w:numPr` reference exists in the exported body. Sourced numbering (including tentative variants) still round-trips fully, and a list created during the session still ships because the body now carries `w:numPr`. Editor.ts: the numbering part is now optional in the export bundle — same shape as appXml / coreXml. TDD - 3 new failing integration tests on blank-doc.docx / Hello docx world.docx (no source numbering, no body refs). - All 3 green after the fix. - The 3 existing SD-2911 tests (active + tentative variants) still pass. Verification: `pnpm --filter super-editor test --run`: 12 698 / 12 698 pass.
…und-trip (SD-3152) (#3308) * fix(super-editor): preserve w:tcMar logical/physical key family on round-trip (SD-3152) Word-authored docx with <w:tcMar><w:start/><w:end/> was gaining duplicate <w:left/><w:right/> children on export. translate-table-cell.js merged attrs.cellMargins (physical LTR-default per SD-3134) into tableCellProperties.cellMargins without reconciling the imported pair, so the v3 tcMar decoder emitted both pairs. Per-side detection now writes the user-visible value back into the imported family: logical-only stays logical, physical-only stays physical, mixed-unchanged preserves both, mixed-edited normalizes to physical (mirrors getTableCellMargins import precedence). New cells default physical. Also sorts tcMar and tblCellMar children into ECMA-376 CT_TcMar / CT_TblCellMar sequence order (top, start, left, bottom, end, right) on decode. Existing tests used arrayContaining and didn't catch the order regression. * test(super-editor): add Word-authored fixture and tests for tcMar key-family round-trip (SD-3152) Real Word-authored docx covering every CT_TcMar / CT_TblCellMar sibling the PR touches: tblCellMar with logical children (§17.4.42), cell 1 tcMar logical-only (§17.4.68 + §17.4.35 + §17.4.10), cell 2 tcMar physical-only (Part 4 §14.3.3 + §14.3.8). Fixture round-trips through Word without validation errors. Three test paths: - Unit fixture test: parses the docx, runs encode + generateTableCellProperties + decode, asserts logical-only stays logical-only, physical-only stays physical-only, schema-order on both translators. - Behavior spec: loads the fixture in headless SuperDoc and confirms logical and physical sources resolve to the same physical padding in an LTR table. - Visual: same fixture uploaded to R2 as rendering/sd-3152-tcmar-key-family.docx for pixel-diff baselines (auto-discovered). * test(super-editor): cover end-to-end exportDocx round-trip and dual-view importer contract (SD-3152) Two gaps remained after the first test pass: 1. exportDocx wiring. The fixture-backed unit test exercised translators and generateTableCellProperties directly, but skipped the editor → exportDocx() pipeline. A wiring regression there would not surface. New behavior spec under tests/behavior/tests/exporting/ loads the fixture, calls editor.exportDocx, unzips the result, and asserts cell 1 emits only logical w:start/w:end, cell 2 emits only physical w:left/w:right, no cell mixes start+left or end+right, and tblCellMar stays schema-ordered. 2. Importer dual-view contract. The original report claimed source start/end info was "not preserved." The data is preserved on attrs.tableCellProperties.cellMargins by design; attrs.cellMargins is a separate LTR-default physical projection per SD-3134. Two new legacy-handle-table-cell-node tests lock that contract so a future change can't quietly collapse the dual view back into a polymorphic shape.
…on (SD-2779) (#3307) * refactor(painter-dom): rename rtl-paragraph feature to inline-direction (SD-2779) The painter-dom feature module previously called `features/rtl-paragraph/` handles two related but distinct OOXML elements: w:pPr/w:bidi (paragraph inline direction) and w:rPr/w:rtl (run inline direction). Both belong to the inline-direction axis. Table visual direction (w:bidiVisual) and writing mode (w:textDirection) are separate orthogonal axes owned elsewhere. Rename the folder + import path + feature-registry entry to `inline-direction` so the module name matches the axis, and add an explicit note (registry comment + index.ts JSDoc) calling out the two orthogonal axes the module does NOT own. Internal file (`rtl-styles.ts`) and exported function names (`applyRtlStyles`, `shouldUseSegmentPositioning`, `isRtlParagraph`, `resolveTextAlign`) keep their RTL framing because they describe RTL detection and styling specifically. Only the feature folder name changes. Tests: painter-dom 1070 pass. Build sweep (`pnpm --filter @superdoc/painter-dom... build`) clean. * docs(direction): correct OOXML spec citations on the orthogonal-axis note §17.18.93 is ST_TextDirection (the enum values), not the element location. w:textDirection lives at §17.3.1.41 (paragraph) and §17.4.72 (cell). Also soften the "inline direction" framing to "paragraph/run inline bidi handling": w:rPr/w:rtl is a run-level bidi/RTL/CS trigger, not strictly a paragraph base-direction signal. Comment-only; no code change.
…3293) * fix(painter-dom): honor appearance:hidden on inline SDTs (SD-3110) ECMA-376 §17.5.2.6 (w15:appearance val="hidden") means the SDT is present in the document for anchoring but visually transparent. Two prior leaks made hidden SDTs anything but: 1. The renderer painted full chrome (padding/border/hover/selected outline) regardless of appearance, leaving a visible bracket around the wrapped span. 2. The alias label was stamped into the DOM as a <span> child whose textContent included the alias. That text leaked into copy-paste (selecting and copying the wrapped phrase pulled in 'Inline content' / 'Harvey citation' / whatever the SDT's alias was) AND into screen-reader output. The data was already correct end-to-end on the converter side: import parses w15:appearance into the node attrs (extractAppearance in handle-structured-content-node.js), and the Document API surfaces it. The gap was that StructuredContentMetadata in @superdoc/contracts didn't carry the field, so the pm-adapter -> renderer bridge stripped it. Four-file fix: - contracts: add appearance?: StructuredContentAppearance to StructuredContentMetadata. - style-engine: read attrs.appearance in normalizeStructuredContentMetadata, validating against the three spec values (boundingBox | tags | hidden); unknown values are dropped rather than poisoning rendering. - painter-dom renderer: createInlineSdtWrapper now stamps data-appearance="hidden" on the wrapper AND skips appending the alias <span> entirely when hidden. - painter-dom styles: CSS rule keyed off [data-appearance='hidden'] zeroes padding/border/border-radius and neutralizes hover and selected states. Tests: - style-engine: appearance carries through, unknown values are dropped, omitted attr stays undefined. - painter-dom: render a hidden inline SDT and assert (a) data-appearance='hidden' is on the wrapper, (b) no .superdoc-structured-content-inline__label child exists, (c) wrapper.textContent equals exactly the wrapped phrase with the alias text nowhere in it. Verified: - @superdoc/painter-dom: 1071/1071 - @superdoc/style-engine: 132/132 - @superdoc/contracts: 232/232 - @superdoc/pm-adapter: 1838/1838 * test(behavior): cover hidden-appearance inline SDT (SD-3110) End-to-end coverage for the painter-dom fix in #3293. Fixture is a 5-paragraph DOCX with inline SDTs across the appearance matrix: hidden, boundingBox, default (omitted), and two adjacent hidden. Five assertions, one per claim the PR makes plus a copy-paste smoke test: - data-appearance="hidden" stamped on hidden wrappers, absent on others - no __label child inside hidden wrappers; present on others - hidden wrappers omit the alias canary from textContent - no hidden-alias canary appears in the painted layout root - selection.toString() over a hidden wrapper returns only the wrapped phrase Visual coverage follow-up: drop a slim variant in tests/visual/test-data/ via pnpm docs:upload (corpus is R2-backed, not in-tree). * fix(painter-dom): keep hidden inline SDTs out of lock-hover styling (SD-3110) Caught in review: the lock-hover rule .superdoc-structured-content-inline[data-lock-mode]:hover:not(.ProseMirror-selectednode) has (0,4,0) specificity, one higher than the hidden-appearance rule's (0,3,0). Hovering a hidden inline SDT therefore re-introduced the blue background and z-index 9999999 boost the rule meant to suppress. Exclude data-appearance="hidden" from the inline branch of the lock-hover rule. Block-level branch is untouched; block hidden isn't a render path yet. Adds a behavior regression assertion: hover a hidden wrapper and verify the computed backgroundColor doesn't pick up the lock-hover blue and z-index doesn't jump to 9999999. 18/18 behavior cases × 3 browsers green; painter-dom unit tests still 1071/1071. * fix(painter-dom): restore viewing-mode hover suppression on inline SDTs (SD-3110) The previous fix added a second chained :not([data-appearance='hidden']) to the inline lock-hover rule, which bumped its specificity from (0,4,0) to (0,5,0). The viewing-mode suppression rule below sits at (0,4,0), so it lost the cascade and the lock-hover blue re-appeared on hover in viewing mode — regressing the SD-2232 behavior test "inline SDT hover does not show background in viewing mode". Collapse the two predicates into a single :not(a, b). Comma-list :not() takes the max specificity of its arguments, not the sum, so the selector stays at (0,4,0), viewing-mode suppression wins again, and the hidden-appearance exclusion is preserved. Verified: 22/22 SDT behavior cases on chromium, 44/44 on firefox+webkit; painter-dom unit tests still 1071/1071.
…(SD-3156) (#3310) * feat(custom-ui): add contentControl as a first-class viewport entity (SD-3156) Wires content controls onto the same viewport surface comments and tracked changes already use, so consumers can hit-test and look up painted rects for the SDT under the cursor without scraping data-sdt-* attrs themselves. Touches the five places that today gate the entity surface to 'comment' | 'trackedChange': - ViewportEntityHit gains a third variant { type: 'contentControl', id, scope?, tag? }. The hit carries only the fields the painter already stamps (data-sdt-id / sdt-scope / sdt-tag); alias / controlType / lockMode require metadata plumbing that doesn't exist on main yet, so consumers wanting full property data call editor.doc.contentControls.get with a ContentControlTarget ({ kind, nodeType: 'sdt', nodeId }). - ViewportGetRectInput.target is widened from @superdoc/document-api EntityAddress (comment | trackedChange) to a UI-local ViewportEntityAddress that adds ContentControlViewportAddress. The Document API's EntityAddress stays narrow — content controls aren't a doc-api navigation primitive (no editor.doc.contentControls. navigateTo), so the union extension lives on the UI surface. - entity-at.ts walks data-sdt-id + data-sdt-type, filtering explicitly to 'structuredContent' so field annotations, document sections, and doc-part objects don't surface through the contentControls.* namespace. Innermost-first ordering matches comment + tracked-change. - ui.viewport.getRect's entity-type allowlist accepts 'contentControl'. - PresentationEditor.getEntityRects routes 'contentControl' to a new findRenderedContentControlElements helper next to the existing comment / tracked-change finders. The helper restricts its selector to the two wrapper classes (INLINE_SDT_WRAPPER / BLOCK_SDT) because the painter also stamps SDT metadata on every child text-run element; a naive [data-sdt-id][data-sdt-type=structuredContent] selector returns wrapper + every run, polluting the single-painted- occurrence contract `rect` / `rects` promises. v1 is body-only. SDT wrappers don't stamp data-story-key today, so the finder accepts storyKey for signature parity but ignores it; a header / footer SDT will still match. JSDoc on the helper documents the limitation and the path forward. Tests: - entity-at.test.ts: 17 cases (inline, block, nested, dedup, non- structuredContent SDT types intentionally ignored, attr-less id ignored, mixed entity stacks, plus two compile-time contract checks asserting ViewportEntityHit and ViewportGetRectInput.target accept the contentControl variant). - EntityRectFinder.test.ts: 8 cases (single, multi-fragment, type filter, empty inputs, attr escaping, classless-element rejection, child-run overmatch regression). Full super-editor suite: 12956/12956 green. * fix(custom-ui): address content-control viewport review findings (SD-3156) Follow-up commit on top of the initial SD-3156 amend addressing four findings from PR #3310 review: 1. Public barrel was missing the new types. `ContentControlViewport Address` and `ViewportEntityAddress` weren't re-exported from `superdoc/ui`, so typed consumers couldn't annotate a content- control `getRect` call even though the runtime accepted it. Added both to `packages/superdoc/src/ui.d.ts`. 2. Wrapper-class scoping needed a regression. The painter stamps SDT metadata on the wrapper AND every child text-run; a naive `[data-sdt-id][data-sdt-type=structuredContent]` selector returns wrapper + every run. The amend already restricted the finder to the two wrapper classes; this commit adds the regression test that would catch a future drift. 3. Cross-story behavior codified. SDT wrappers don't stamp `data-story-key` today, so the `storyKey` argument on the finder is a no-op — a content control with the same id in body and header matches both. New unit test asserts that v1 behavior so a future change can't silently narrow it without an intentional test update. JSDoc on the helper now points to SD-3155 (umbrella) for the follow-up. 4. Test helper used hardcoded class strings. Swapped to `DOM_CLASS_NAMES.INLINE_SDT_WRAPPER` / `BLOCK_SDT` so a future rename can't silently de-sync the helper from production. Barrel regression test uses a file-scan strategy: vitest strips types at runtime and the workspace tsc config excludes `*.test.ts`, so a type-import-only approach wouldn't catch the regression. The scan asserts the four relevant `type` names appear in the export list. I verified the test fails when the barrel is reverted. Verified locally: super-editor suite 12957 pass (+1 cross-story case), superdoc suite 988 pass (+4 barrel cases). Live in custom-ui demo against the SD-3110 fixture: broad selector returns 2 matches per SDT (wrapper + child run), wrapper-class selector returns 1; injected header-story wrapper for SDT 1001 returns both body and header matches as documented. Part of SD-3155.
Co-authored-by: Gabriel Chittolina <gabrielchittolina1@gmail.com>
Narrower variant of SD-3152: a w:tcMar with ONLY w:start/w:end (no top/bottom). Exercises the vertical-side branch of the merge loop being fully skipped, while the horizontal branch still has to preserve the logical key family. Fixture is built by the word-api ooxml-fixture SDK CLI (build --type tc-mar-logical) and pre-validated with OpenXmlValidator (Office2019 profile). Same source the fixture-feedback harness uses, so the test exercises identical input. The original SD-3158 report was a stale-dist false positive in fixture-feedback (packages/superdoc/dist is gitignored and was pre-fix). After rebuild, the SDK fixture exports cleanly. This test pins the behavior so a future regression on the start/end-only branch won't slip through CI.
…rtEntityAddress (SD-3156) (#3316) The public `superdoc/ui` barrel (`packages/superdoc/src/ui.d.ts`) imports both names from `@superdoc/super-editor/ui`, but they were missing from `packages/super-editor/src/ui/index.ts`. The existing `ui.barrel.test.ts` only string-scans the barrel file, so it didn't catch the upstream gap. CI's consumer-typecheck matrix (`bundler | node16 | nodenext` with `skipLibCheck=false`) was failing with TS2305 on both names since #3310 landed.
Co-authored-by: Caio Pizzol <97641911+caio-pizzol@users.noreply.github.com>
* fix(super-editor): restore toolbar dropdown keyboard focus * fix(super-editor): bubble Enter from dropdown trigger so it reopens after Escape After Escape restored focus to the inner ToolbarButton (.toolbar-item), @keydown.enter.stop on that element swallowed Enter before ButtonGroup's roving-tabindex handler could see it. Result: pressing Enter on the restored focus did nothing - the dropdown was unable to be reopened by keyboard until focus moved elsewhere. New prop `allowEnterPropagation` on ToolbarButton. When true (set by ButtonGroup's dropdown branch), the keydown handler does not stopPropagation, so Enter bubbles to .toolbar-item-ctn where activateToolbarItem reopens the dropdown. Plain buttons keep the default (false) so they do not double-fire their command emission. Split buttons (Bullet list / Numbered list main) are unaffected: handleSplitMainClick stops propagation internally, so Enter still runs the main command and does not toggle the dropdown. Tests added in ButtonGroup.test.js exercising the real ButtonGroup + ToolbarButton + ToolbarDropdown stack: - Enter on inner .toolbar-item opens the dropdown - Escape-then-Enter on restored focus reopens the dropdown - Space on inner .toolbar-item opens the dropdown - Split button: Enter runs the main command, dropdown stays closed --------- Co-authored-by: Caio Pizzol <caio@superdoc.dev> Co-authored-by: Caio Pizzol <97641911+caio-pizzol@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Caio Pizzol <97641911+caio-pizzol@users.noreply.github.com>
|
📖 Docs preview: https://superdoc-nick-merge-main-stable-33.mintlify.app |
There was a problem hiding this comment.
💡 Codex Review
superdoc/.github/workflows/promote-stable-docs.yml
Lines 83 to 85 in b572d54
On the automatic workflow_run path, a successful stable SuperDoc release creates the v* tag on github.event.workflow_run.head_sha itself. Since git tag --merged <commit> lists tags merged into that commit, the newly-created tag is included in tags_at_head after git fetch --tags, so comm -23 is empty and docs-stable never advances for real releases.
ℹ️ 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".
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
🎉 This PR is included in superdoc-cli v0.10.0 The release is available on GitHub release |
|
🎉 This PR is included in superdoc v1.33.0 The release is available on GitHub release |
|
🎉 This PR is included in @superdoc-dev/react v1.4.0 The release is available on GitHub release |
|
🎉 This PR is included in vscode-ext v2.5.0 |
|
🎉 This PR is included in superdoc-sdk v1.9.0 |
|
🎉 This PR is included in @superdoc-dev/mcp v0.5.0 The release is available on GitHub release |
No description provided.