Skip to content

Run the Mermaid quality pass and ship gates #530

Closed

Description

@HMarzban

Part of #521. Depends on #529. Read the tracker first: its §Shared contract binds this issue. Work on main.

Summary

Run the house DRY and KISS pass and take the code-janitor flag. Then sweep layering and dead files on the final code, and audit the docs against the code. Finish with check:ci and the Docker cold build.

Rulings: 8 (#521 §Maintainer rulings).

Files and interfaces

Files

  • Modify only on a failed check: files earlier tasks created or changed. Create no new repo file.
  • Modify: the ship-check file.

Implementation steps

  • Step 1: Run the code-janitor skill on PKG plus the webapp and server files from Draw Mermaid diagrams in the pad, history and /editor #528. Tell it:

    Record its verdict word for word in ## Quality pass. Pass needs the code-janitor flag granted and no BLOCK finding left.

  • Step 2: Sweep layering and dead files (after the janitor, so it checks the final code).

    • git -C REPO ls-files --others --exclude-standard lists only new files from the file map: no LICENSE, tsconfig.test.json, examples/vanilla/style.css or scratch file.
    • git -C REPO grep --untracked -nE "from ['\"]pele|import\(['\"]pele" -- 'extensions/**/*.ts' 'apps/**/*.ts' 'apps/**/*.tsx' 'packages/**/*.ts': every hit is in createRenderer.ts, and exactly one is the runtime import('pele/lazy').
    • No export * in PKG/src. Rerun the Add the Mermaid renderer, default view, styles and node view #524 Step 7 layering greps. types.ts imports only from @tiptap/*, and constants.ts at most import type from ./types.
    • In PKG, the orphan check prints nothing: for f in $(find src -name '*.ts' ! -name index.ts); do b=$(basename $f .ts); grep -rqE "from '[./]+.*$b'" src || echo "orphan $f"; done.
    • The d.ts greps ([Tracking] Mermaid diagrams in docs.plus through a headless Pele extension #521 §Global constraints → Pele).
    • git -C REPO grep -n "extension-mermaid\|insertMermaid" -- apps/webapp/src/components/chatroom and git -C REPO diff --stat -- apps/webapp/src/stores print nothing.
  • Step 3: Recheck after janitor edits. If the janitor changed any src/ or webapp file, rebuild and run bun run test in PKG. Then rerun Run the Mermaid browser pass and security probes #529 Step 1, so dev is running. Run browser.ts type-race offline, and repeat Step 2.

  • Step 4: Audit the docs against the code. This runs after the janitor, because its documentation chain rewrites prose. It edits docs only.

    • Check each factual claim in the package README, CONTRIBUTING, docs/*.md and cypress/e2e/README.md, plus docs/diagrams.md, the CONTEXT.md entry, the design-system row and the new family rows, against a code line. Check docs/security.md against the probe rows. Fix false claims and record each fix.
    • Rollback check: start bun run playground in PKG in the background, and stop it afterwards. In the playground console, after _editor.commands.setContent('<p>Intro</p><pre><code class="language-mermaid">flowchart LR\n A --> B</code></pre>'), run:
      const json = _editor.getJSON()
      const bare = new _editor.constructor({ extensions: _editor.extensionManager.extensions.filter((e) => !['mermaid', 'starterKit'].includes(e.name)), content: json })
      const result = [JSON.stringify(bare.getJSON()) === JSON.stringify(json), bare.getHTML().includes('<code class="language-mermaid">'), bare.extensionManager.extensions.some((e) => e.name === 'mermaid')]
      bare.destroy()
      result
      Expected: [true, true, false], with no duplicate-extension warning.
    • Repeat the credit-line grep, the caveat parity loop and the quickstart sync check (Write the extension-mermaid README, docs, assets and StackBlitz example #526 Step 10).
  • Step 5: Format untracked files. cd REPO && git ls-files --others --exclude-standard -z | xargs -0 bunx prettier --check --ignore-unknown --ignore-path REPO/.prettierignore -- prints All matched files use Prettier code style!.

  • Step 6: Run check:ci. Stop make dev-local and any playground. bun REPO/scripts/check-next-server.ts exits 0. Run bun run check:ci from REPO.

    • The header ext line lists extension-mermaid.
    • webapp build:ci reads pass. A skip there means a Next server was live and the build never ran.
    • The last line is PASS every run gate passed. Named skips are not implied green on GitHub.
    • Copy the verdict block into ## check:ci verdict.
  • Step 7: Run the Docker cold build. make build-prod-ci from REPO. Expected last line: ✅ Production build validation passed. Record the result as a row in ## Measure and release gates. If the Docker daemon is not running, add "make build-prod-ci before the first (build): push" to Owed. That is not a pass.

  • Step 8: Clean up. Run rm -f REPO/apps/webapp/public/sw.js REPO/apps/webapp/public/workbox-*.js REPO/apps/webapp/public/worker-*.js.

  • Step 9: Loop rule. The flag and the verdicts must describe the final code. A code fix after Step 1 goes back through Steps 1 to 8. A doc-only fix after Step 4 goes back through Steps 5 and 6.

  • Step 10: Report on the tracker. Hand the verdict block (janitor flag, check:ci, Docker) to the maintainer to post on [Tracking] Mermaid diagrams in docs.plus through a headless Pele extension #521.

  • Step 11: Review checkpoint. Report the janitor verdict, the sweep, the doc audit, the check:ci block, the Docker result, and everything Owed.

Acceptance criteria

  • code-janitor grants its flag with no BLOCK finding.
  • check:ci ends with the PASS line on the final code, with webapp build:ci reading pass.
  • make build-prod-ci passes, or is recorded as owed because the Docker daemon was down.

Activity

  1. HMarzban commented on Oct 19, 2026

    @HMarzban
    CollaboratorAuthor

    Maintainer ruling, 2026-10-19

    The source split is accepted: nodeView/fold.ts, nodeView/renderLoop.ts, view/icons.ts, view/element.ts, nodeView/copy.ts, nodeView/theme.ts, src/hashSource.ts, and internal types in src/renderer/types.ts. #521 §File map is updated to match.

    The work is in the maintainer's working tree and is not committed or pushed yet.

  2. HMarzban commented on Oct 19, 2026

    @HMarzban
    CollaboratorAuthor

    Ship gates, progress 2026-10-19

    • Pass:
    • Fixed: the limit test imported the main pele entry. It now uses pele/lazy (1258a9d42).
    • Ruled 2026-10-19: the three CSS lines flagged by the sweep are accepted: :focus-visible, plus .pele-zoom and contain: none inside @media print. The print lines are what make the package print correctly.
    • Still owed:
      • A fresh review pass on 7547ee82e (limit advice, print rules, runtime theme guard), because the earlier pass came before it.
      • The rollback check on the playground.
      • Docker on the final tree.
      • check:ci, which the maintainer runs.
      • The Node 20.19 CJS run (Register extension-mermaid as the sixth family package #527).
  3. HMarzban commented on Oct 19, 2026

    @HMarzban
    CollaboratorAuthor

    Step 4 rollback check, 2026-10-19 (HEAD 7ca925101)

    The rollback passes. With the Mermaid extension removed, the document keeps the same JSON, the HTML keeps <code class="language-mermaid">, and no mermaid extension is left: [true, true, false], with no duplicate warning. The result is the same as the earlier run. Commits 7547ee82e and 17b96cc3b changed nothing here.

    The snippet in Step 4 crashes as written. new Editor adds the 10 core extensions again, so RangeError: Adding different instances of a keyed plugin (focusEvents$). Use this filter:

    const core = ['editable','clipboardTextSerializer','commands','focusEvents','keymap','tabindex','drop','paste','delete','textDirection']
    const bare = new _editor.constructor({
      extensions: _editor.extensionManager.extensions.filter(
        (e) => !['mermaid', 'starterKit', ...core, 'listItemBranchingDeleteKeymap'].includes(e.name)
      ),
      content: json
    })

    Dropping listItemBranchingDeleteKeymap removes a duplicate warning that does not come from Mermaid. In @tiptap/extension-list 3.31.3, ListItem.addExtensions() adds that child again, and the control run without the Mermaid change shows the same warning.

    Log: Notes/local-docs/mermaid-ship-check/rollback-head-7ca925101.log (local). Step 4's three doc checks were not part of this run.

  4. HMarzban commented on Oct 21, 2026

    @HMarzban
    CollaboratorAuthor

    Fresh review pass on 7547ee82e, 17b96cc3b, 494af7123 and aebd55866, 2026-10-21

    A strict maintainability review, run as the thermo-nuclear reviewer agent. Its rubric skill refused to load (disable-model-invocation), so the agent used its stated fallback: a strict maintainability audit.

    Verdict: GRANTED_WITH_NOTES. Nothing blocks the release.

    Checked and correct:

    • limitMessage.ts maps each of Pele 1.7.0's seven limit messages, and the test passes on 1.7.0.
    • LIMITS in checkDiagrams.ts (15000/500) matches DEFAULT_LIMITS.
    • ignoreMutation touches only the frame's style attribute.
    • The print selectors match the DOM.

    Findings:

    1. FIX, now fixed locally (uncommitted). apps/hocuspocus.server/src/modules/mcp/domain/checkDiagrams.ts told every limit error "over 15000 characters or 500 edges". It still did after 17b96cc3b fixed the same thing in the extension. Before: block-beta\n columns 1\n space:1000000 (38 characters) got the size sentence. After: "Pele refused it: A block diagram can hold at most 100000 space blocks." The size and edge cases keep the old sentence, and the 14-keyword smoke test still passes.
    2. NOTE, fixed locally (uncommitted). extensions/extension-mermaid/docs/api.md said a limit message is always the extension's sentence, which contradicts the next lines. It now says "usually".
    3. NOTE, left for later. edge-a11y-theme.cy.ts: the helper paragraphTexts() ends in .then, so it never retries, and 494af7123 copied its body inline. Make the helper retry, then drop the copy.
    4. NOTE, left as is. For the four limits that keep Pele's text, the headline still reads "This diagram is too large to draw." That is loosely true.

    check:ci passed on the final code in the pre-push run for 3784c2c65, and that same tree built and deployed (run 39398067267). The two fixes above need one commit and a (build): back.

  5. HMarzban commented on Oct 21, 2026

    @HMarzban
    CollaboratorAuthor

    Findings 1 and 2 above are committed in ae692d54d and deployed to production on 2026-10-21 (deploy tip 0130027bf). It also adds checkDiagrams.test.ts, which fails on the old code. Findings 3 and 4 stay as noted.

  6. HMarzban commented on Oct 21, 2026

    @HMarzban
    CollaboratorAuthor

    Status, 2026-10-21

    • make build-prod-ci passes. All four production images build: webapp, admin, rest-api and hocuspocus. It first failed after Require a password on production Redis #438, because the build stub had no REDIS_PASSWORD. 5cb91e273 fixed that.
    • check:ci passes on every push today, with webapp build:ci reading pass.
    • code-janitor, group G1 (the package): four comment-only edits, no runtime change, and all gates pass (shipped in d6a09faa1). Five file-name renames were proposed and skipped, because CONTEXT.md names those modules by their current file names. G2 (the webapp wiring), the production-readiness sweep and the supervisor step are still owed.

    Flaky test, seen in CI on 2026-10-21. edge-a11y-theme.cy.ts › "keeps diagram zoom keys and the editor caret apart on a fine pointer" failed once in run 39597105751, then passed on the re-run:

    AssertionError: Timed out retrying after 6000ms: expected [ Array(82) ] to include 'AfZter'
      at edge-a11y-theme.cy.ts:445
    

    The typed Z never reached the "After" paragraph. caretInParagraph already waits for .ProseMirror focus, and 494af7123 already retries the read, so the key itself is lost. The likely cause is that the editor is not ready for real keys at that moment. Finding 3 above names the same file. That case is not covered by the focus wait: the Wide diagram finishes rendering, or the 300 ms redraw debounce fires, between the caret set and the key. Fix it before 2.2.0 ships. Wait for the diagram to settle and for the selection to sit inside "After" before realType, then prove the fix by sabotage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions