Skip to content

Conversation

@buger
Copy link
Contributor

@buger buger commented Oct 28, 2025

This PR refines the flowchart validator’s heuristic that flags parentheses inside unquoted square‑bracket node labels (FL-LABEL-PARENS-UNQUOTED).

Why

  • Parentheses in unquoted [...] labels are ambiguous for Mermaid; we surface them explicitly for better DX and autofix.
  • The previous sweep could double‑report or miss labels when multiple [...] segments or quotes were present on a line.

What changed

  • Quote‑aware, nested‑bracket scan per line to find the correct matching ].
  • Deduplication using per‑line start/end ranges to avoid repeating diagnostics already mapped by the parser.
  • Keeps exclusions for typed shapes and ( … )‑wrapped content inside [].

Effect

  • Correctly flags A[initialize()] only when the label is truly unquoted, even in lines with multiple labels and quotes.
  • Reduces false positives and duplicate entries.

Review aids

  • Build and run per‑type comparison for flowcharts:
    • npm run build && npm run compare:renderers:flowchart
    • Inspect .tmp-compare-all/flowchart/<case>/diff.json and summaries.

Supersedes

…nested + quoted-safe)\n\n- Track label range per [ ... ] segment to dedupe diagnostics\n- Scan with nested-bracket depth; skip brackets inside quotes and escapes\n- Keep skipping typed shapes ([/…/], [\…\]) and ( … )-wrapped labels\n- Prevent duplicate FL-LABEL-PARENS-UNQUOTED when parser already mapped same segment\n\nThis tightens parity with Mermaid by flagging method-like labels such as A[doWork()] only when truly unquoted, even in lines with multiple labels or quotes.\n

(cherry picked from commit 43ffc1a)
@probelabs
Copy link

probelabs bot commented Oct 28, 2025

🔍 Code Analysis Results

[...]

🐛 Debug Information

Provider: google
Model: default
API Key Source: GOOGLE_API_KEY
Processing Time: 607414ms
Timestamp: 2025-11-03T08:44:04.671Z
Prompt Length: 1120357 characters
Response Length: 2722 characters
JSON Parse Success:

Debug Details

⚠️ Debug information is too large for GitHub comments.
📁 Full debug information saved to artifact: visor-debug-2025-11-03T08-44-06-220Z.md

🔗 Download Link: visor-debug-95
💡 Go to the GitHub Action run above and download the debug artifact to view complete prompts and responses.


Powered by Visor from Probelabs

Last updated: 2025-11-03T08:44:06.388Z | Triggered by: synchronize | Commit: 2c4a530

💡 TIP: You can chat with Visor using /visor ask <your question>

@probelabs
Copy link

probelabs bot commented Oct 28, 2025

🔍 Code Analysis Results

Security Issues (1)

Severity Location Issue
🟡 Warning scripts/generate-previews.js:85
A command injection vulnerability exists in the `runMermaidCli` function. The shell command is constructed using a template string that includes the `filepath` variable without proper sanitization. If a file with a malicious name (e.g., `"; touch pwned; echo ".mmd`) is present in the test fixtures, it could lead to arbitrary command execution when this script is run. While this is a developer script, it poses a risk in an open-source environment where pull requests may introduce such files.
💡 SuggestionUse `child_process.execFileSync` instead of `execSync`. Pass the command and its arguments as an array to `execFileSync` to avoid shell interpretation of special characters in file paths. The arguments array should be constructed safely, for example: `execFileSync('npx', ['@mermaid-js/mermaid-cli', '-i', filepath, '-o', outSvg, ...])`.

Architecture Issues (1)

Severity Location Issue
🟡 Warning src/diagrams/flowchart/validate.ts:162-179
The logic for handling quotes and escape characters is duplicated across two loops.
💡 SuggestionRefactor the duplicated logic for scanning through quoted strings (lines 162-168 and 173-179) into a shared helper function. This will improve maintainability and reduce the risk of introducing bugs if the quote-handling logic needs to be changed in the future. For example, a function like `findMatchingBracket(line: string, startIndex: number): number` could encapsulate this logic, handling quotes and nested brackets internally.

✅ Performance Check Passed

No performance issues found – changes LGTM.

Quality Issues (1)

Severity Location Issue
🟢 Info src/diagrams/flowchart/validate.ts:162-176
The logic for handling quotes and escape characters is duplicated between the outer loop (lines 162-166) and the inner loop for finding a matching bracket (lines 170-176). This increases complexity and makes the code harder to maintain.
💡 SuggestionRefactor the duplicated quote-handling logic into a separate helper function. This function could take the current character and state (e.g., `{ inQuote: boolean, isEscaped: boolean }`) and return the updated state, reducing code duplication and improving the readability of the main loops.
🐛 Debug Information

Provider: google
Model: default
API Key Source: GOOGLE_API_KEY
Processing Time: 607414ms
Timestamp: 2025-11-03T08:44:04.671Z
Prompt Length: 1120357 characters
Response Length: 2722 characters
JSON Parse Success:

Debug Details

⚠️ Debug information is too large for GitHub comments.
📁 Full debug information saved to artifact: visor-debug-2025-11-03T08-44-09-042Z.md

🔗 Download Link: visor-debug-95
💡 Go to the GitHub Action run above and download the debug artifact to view complete prompts and responses.


Powered by Visor from Probelabs

Last updated: 2025-11-03T08:44:09.213Z | Triggered by: synchronize | Commit: 2c4a530

💡 TIP: You can chat with Visor using /visor ask <your question>

buger added 13 commits October 28, 2025 20:57
…timeout + retry; add compat gap for interactions-linkstyle-multi\n\n- Flowchart VALID/INVALID pages updated\n- PREVIEW_MERMAID_CLI_TIMEOUT_MS env support + one retry to reduce flakes\n- Mark interactions-linkstyle-multi.mmd as compat (mermaid-cli intermittently reports INVALID)
…s compat gap for member-without-name\n\n- Refresh INVALID_DIAGRAMS for class and sequence\n- Mark class member-without-name.mmd as compat since mermaid-cli rejects unnamed members
…ncode via autofix\n\n- Remove heuristic skip for parallelogram/trapezoid labels in validator sweep\n- Stop early-continue in semantics so RoundOpen/RoundClose under [/…/] still map to FL-LABEL-PARENS-UNQUOTED\n- Ensures cases like API1[/Streams API\n(/streams-api)/] are fixed by encoding parens (quotes unsupported in typed labels)\n- Keeps skipping pure ( … )-wrapped labels to avoid flagging shape parens themselves
…s\n\n- diamond-indexof-brackets.mmd → shows decision label with parens and bracket chars; expect FL-LABEL-PARENS-UNQUOTED x2\n- typed-parallelogram-parens.mmd → [/…/] label with parens; expect FL-LABEL-PARENS-UNQUOTED; autofix encodes &#40; &#41;\n- subgraph-unquoted-title.mmd → unquoted subgraph title with spaces; expect FL-SUBGRAPH-UNQUOTED-TITLE
…ing required for special characters\n\n- Parser: accept Identifier + additional words (Identifier/Text/Number) as subgraph title\n- Diagnostics: only emit FL-SUBGRAPH-UNQUOTED-TITLE when hazards like [](){}:/\ or quotes appear\n\ntests(flowchart): add fixtures for new validations\n- diamond-indexof-brackets.mmd (parens in unquoted decision with bracket chars)\n- typed-parallelogram-parens.mmd (parens inside [/…/] label → encode via autofix)\n- remove subgraph-unquoted-title (now valid); update expected-errors\n\nchore(previews): regenerate flowchart INVALID_DIAGRAMS.md
…anges\n\n- Update flowchart rendered SVGs for subgraph examples\n- Fix sequence valid fixture (blocks-alt-opt-loop.mmd) and regenerate VALID_DIAGRAMS.md
…-fixtures/sequence/invalid-compat.json with autonumber-malformed.mmd\n- generator: support invalid-compat.json to skip failing --fix=all validations for listed files
…nerate ALL invalid previews\n\n- scripts/generate-previews.js: reduce CLI errors to a single stable line (Parse error… or Syntax error in text), strip stacks\n- handle invalid-compat in main to avoid scope errors\n- regenerate class/flowchart/pie/sequence INVALID_DIAGRAMS.md with canonical messages
…tofix\n\n- Extend robust label sweep to detect '@' hazard (skip typed [/…/] where quotes unsupported)\n- Dedupe with existing range logic; collect both PARENS/AT codes\n\ntests(flowchart): add 'label-with-at-slash.mmd' invalid fixture + expected error\n- Demonstrates '@' + '/' in label; expect FL-LABEL-AT-IN-UNQUOTED\n
…e' and unknown-type); regenerate flowchart invalid page; ensure '@' row shows ✅ safe
@buger buger merged commit f0024fb into main Nov 3, 2025
9 checks passed
@buger buger deleted the fix/flowchart-validate-parens-sweep branch November 3, 2025 09:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants