Skip to content

Conversation

@buger
Copy link
Contributor

@buger buger commented Nov 27, 2025

Summary

  • flowchart validator: skip FL-LABEL-QUOTE-IN-UNQUOTED when the label is already single-quoted, so we don’t double-fix FL-LABEL-DOUBLE-IN-SINGLE
  • expected errors updated; attribute-quotes case stays covered
  • regenerated flowchart INVALID_DIAGRAMS (47 cases) with stable autofix table

Testing

  • npm run test:errors:flowchart
  • node scripts/test-fixes.js
  • PREVIEW_MERMAID_CLI_TIMEOUT_MS=120000 node scripts/generate-previews.js flowchart --invalid-only

…hen sweeping for quotes\n\n- Skip FL-LABEL-QUOTE-IN-UNQUOTED for single-quoted labels to prevent double fixes\n- Keeps targeted FL-LABEL-DOUBLE-IN-SINGLE handling intact\n\ntests(flowchart): adjust expected errors; add attribute-quotes case stays covered\n\npreviews(flowchart): regenerate INVALID_DIAGRAMS (47 cases) with stable autofix table
@probelabs
Copy link

probelabs bot commented Nov 27, 2025

{
"text": "This PR fixes a bug in the flowchart validator that incorrectly flagged FL-LABEL-QUOTE-IN-UNQUOTED for labels that were already correctly enclosed in single quotes but contained double quotes. This change prevents a "double-fix" scenario, improving the accuracy and reliability of the linter and its auto-fix suggestions.

Files Changed Analysis

  • src/diagrams/flowchart/validate.ts: The core logic change is here. A new isSingleQuoted check is added to the validation logic to ensure the FL-LABEL-QUOTE-IN-UNQUOTED error is not triggered for labels already wrapped in single quotes.
  • test-fixtures/flowchart/INVALID_DIAGRAMS.md: This auto-generated file, which contains previews of invalid diagrams, has been updated to reflect the corrected validation logic and autofixes.
  • test-fixtures/flowchart/expected-errors.json: The expected errors for the quotes-double-inside-single.mmd test case have been updated to remove the FL-LABEL-QUOTE-IN-UNQUOTED error, aligning the tests with the corrected logic.

Architecture & Impact Assessment

  • What this PR accomplishes: It improves the accuracy of the flowchart linter by eliminating a specific false-positive error, leading to more reliable validation and auto-fixing.
  • Key technical changes introduced: The primary change is the addition of a condition to the validateFlowchart function that checks if a label is single-quoted before flagging it for containing unquoted double quotes.
  • Affected system components: The change is confined to the flowchart validation and linting module. No other diagram types or cor```mermaid
    graph TD
    A[Flowchart Validator] --> B["Label contains ""]
    B -- Yes --> C["Is label wrapped in ""?"]
    C -- No --> D["Is label wrapped in ''?"]
    subgraph "Old Logic"
    D -- No --> E[Throw FL-LABEL-QUOTE-IN-UNQUOTED]
    end
    subgraph "New Logic (This PR)"
    D -- Yes --> F[Validation Passes]
    end
    C -- Yes --> F
    end
    C -- Yes --> F

Scope Discovery & Context Expansion

The scope of this change is well-contained within the flowchart diagram processor. The error code FL-LABEL-QUOTE-IN-UNQUOTED and related validation logic are specific to flowcharts, as indicated by the FL- prefix and confirmed by code search. There is no evidence that this fix needs to be applied to other diagram types, as their label parsing rules differ.",
"tags": {
"review-effort": 1,
"label": "bug"
}
}


Powered by Visor from Probelabs

Last updated: 2025-11-28T07:39:20.607Z | Triggered by: pr_updated | Commit: 1afe542

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

@probelabs
Copy link

probelabs bot commented Nov 27, 2025

✅ Security Check Passed

No security issues found – changes LGTM.

✅ Architecture Check Passed

No architecture issues found – changes LGTM.

Performance Issues (1)

Severity Location Issue
🟡 Warning src/diagrams/flowchart/validate.ts:204
The regular expression `/^'[^]*'$/` is used to check if a string is enclosed in single quotes. This check is performed inside nested loops during a heuristic sweep. For this specific check, using `startsWith` and `endsWith` is generally more performant and readable, as it avoids the overhead of the regex engine for a simple prefix/suffix check. In a performance-sensitive context like a linter sweep, avoiding regex for simple string operations is a recommended practice.
💡 SuggestionReplace the regex test with `trimmed.length >= 2 && trimmed.startsWith("'") && trimmed.endsWith("'")`. This is often faster for simple boundary checks and can be easier for other developers to understand at a glance.
🔧 Suggested Fix
const isSingleQuoted = trimmed.length >= 2 && trimmed.startsWith("'") && trimmed.endsWith("'");

Quality Issues (1)

Severity Location Issue
🟡 Warning src/diagrams/flowchart/validate.ts:197
The regex pattern `[^]*` is used to match any character. This is a non-standard and obscure JavaScript-specific syntax that can harm readability and maintainability. For matching content within a single line, the `.*` pattern is functionally equivalent and much more widely understood. Using a standard pattern makes the code easier to understand for future maintainers.
💡 SuggestionReplace the obscure `[^]*` pattern with the standard `.*` for improved readability, as the string being tested does not contain newlines.
🔧 Suggested Fix
const isSingleQuoted = /^'.*'$/.test(trimmed);

Powered by Visor from Probelabs

Last updated: 2025-11-28T07:39:23.279Z | Triggered by: pr_updated | Commit: 1afe542

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

@buger buger merged commit ecb47eb into main Nov 28, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants