refactor: improve text sanitization and alias generation in ER diagram builder#1785
Conversation
📝 WalkthroughWalkthroughThe ChangesMermaid Sanitization and Helper Refactoring
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR refactors Mermaid ER diagram generation to improve identifier/text sanitization and to generate safer aliases/attribute words (including handling some Mermaid-reserved words and marker keyword collisions).
Changes:
- Replaces quote-escaping with
sanitizeQuotedText()and applies it to entity labels, FK labels, and column comments. - Introduces
toEntityAlias()/toAttributeWord()to generate Mermaid-safe entity aliases and attribute words (prefixing when reserved/invalid). - Adds reserved-word sets for Mermaid entities and attribute marker keywords.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| function escapeQuotes(value: string): string { | ||
| return value.replace(/"/g, "'"); | ||
| function sanitizeQuotedText(value: string): string { | ||
| return value.replace(/"/g, "'").replace(/[\r\n\t]+/g, ' '); |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
backend/src/entities/connection/utils/build-mermaid-er-diagram.util.ts (3)
142-148: 💤 Low valueConsider adding a fallback for empty sanitized identifiers.
If the input contains only special characters,
sanitizeIdentifierreturns an empty string, resulting int_as the alias. While syntactically valid, this is not very descriptive.Consider using a more meaningful fallback like
t_tablefor empty cases:function toEntityAlias(value: string): string { const sanitized = sanitizeIdentifier(value); - if (sanitized.length === 0 || /^[0-9]/.test(sanitized) || MERMAID_ENTITY_RESERVED_WORDS.has(sanitized)) { + const base = sanitized.length === 0 ? 'table' : sanitized; + if (sanitized.length === 0 || /^[0-9]/.test(base) || MERMAID_ENTITY_RESERVED_WORDS.has(base)) { - return `t_${sanitized}`; + return `t_${base}`; } - return sanitized; + return base; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/entities/connection/utils/build-mermaid-er-diagram.util.ts` around lines 142 - 148, The toEntityAlias function returns a bare "t_" when sanitizeIdentifier yields an empty string; update toEntityAlias (which calls sanitizeIdentifier and checks MERMAID_ENTITY_RESERVED_WORDS) to provide a meaningful fallback for empty sanitized identifiers (e.g., return "t_table" or "t_<original-safe-name>") instead of "t_"; keep the existing numeric and reserved-word checks and ensure the fallback is only used when sanitized.length === 0 so callers of toEntityAlias receive a descriptive alias.
162-164: 💤 Low valueConsider collapsing consecutive spaces for cleaner output.
The current regex replaces sequences of
\r\n\twith a single space, but mixed whitespace like\n(newline followed by space) produces double spaces. Consider normalizing all whitespace runs:function sanitizeQuotedText(value: string): string { - return value.replace(/"/g, "'").replace(/[\r\n\t]+/g, ' '); + return value.replace(/"/g, "'").replace(/\s+/g, ' ').trim(); }This collapses all consecutive whitespace (including spaces) and trims leading/trailing spaces.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/entities/connection/utils/build-mermaid-er-diagram.util.ts` around lines 162 - 164, sanitizeQuotedText currently only collapses CR/LF/TAB but leaves mixed runs like "\n " producing double spaces; update the function (sanitizeQuotedText) to first replace double quotes with single quotes, then normalize all whitespace runs by replacing /\s+/ with a single space and trim the result so leading/trailing spaces are removed, ensuring cleaner, collapsed whitespace in the returned string.
150-156: 💤 Low valueConsider adding a fallback for empty sanitized identifiers.
Similar to
toEntityAlias, if the input sanitizes to an empty string, this returns just_. Consider a more descriptive fallback:function toAttributeWord(value: string): string { const sanitized = sanitizeIdentifier(value); - if (sanitized.length === 0 || /^[0-9]/.test(sanitized) || MERMAID_ATTRIBUTE_KEY_WORDS.has(sanitized)) { + const base = sanitized.length === 0 ? 'attr' : sanitized; + if (sanitized.length === 0 || /^[0-9]/.test(base) || MERMAID_ATTRIBUTE_KEY_WORDS.has(base)) { - return `_${sanitized}`; + return `_${base}`; } - return sanitized; + return base; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/entities/connection/utils/build-mermaid-er-diagram.util.ts` around lines 150 - 156, toAttributeWord currently returns just "_" when sanitizeIdentifier(value) yields an empty string; update the function so empty sanitized identifiers get a descriptive fallback instead of a bare underscore. In function toAttributeWord, change the empty-case branch to return a clear fallback like `_attribute` (or better `_attribute_<shortHash>` computed from the original value for uniqueness) rather than `_`; you can add a small helper (e.g., hashString) to produce a short hex/hash suffix if needed to avoid collisions while keeping the result a valid identifier.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@backend/src/entities/connection/utils/build-mermaid-er-diagram.util.ts`:
- Around line 142-148: The toEntityAlias function returns a bare "t_" when
sanitizeIdentifier yields an empty string; update toEntityAlias (which calls
sanitizeIdentifier and checks MERMAID_ENTITY_RESERVED_WORDS) to provide a
meaningful fallback for empty sanitized identifiers (e.g., return "t_table" or
"t_<original-safe-name>") instead of "t_"; keep the existing numeric and
reserved-word checks and ensure the fallback is only used when sanitized.length
=== 0 so callers of toEntityAlias receive a descriptive alias.
- Around line 162-164: sanitizeQuotedText currently only collapses CR/LF/TAB but
leaves mixed runs like "\n " producing double spaces; update the function
(sanitizeQuotedText) to first replace double quotes with single quotes, then
normalize all whitespace runs by replacing /\s+/ with a single space and trim
the result so leading/trailing spaces are removed, ensuring cleaner, collapsed
whitespace in the returned string.
- Around line 150-156: toAttributeWord currently returns just "_" when
sanitizeIdentifier(value) yields an empty string; update the function so empty
sanitized identifiers get a descriptive fallback instead of a bare underscore.
In function toAttributeWord, change the empty-case branch to return a clear
fallback like `_attribute` (or better `_attribute_<shortHash>` computed from the
original value for uniqueness) rather than `_`; you can add a small helper
(e.g., hashString) to produce a short hex/hash suffix if needed to avoid
collisions while keeping the result a valid identifier.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1760512f-08f6-456d-827c-0d45740fedf4
📒 Files selected for processing (1)
backend/src/entities/connection/utils/build-mermaid-er-diagram.util.ts
Summary by CodeRabbit
Release Notes
Bug Fixes
Refactor