codeant-ai-for-open-source[bot] commented on code in PR #37229:
URL: https://github.com/apache/superset/pull/37229#discussion_r3618959561
##########
superset-frontend/plugins/plugin-chart-echarts/src/utils/forecast.ts:
##########
@@ -26,6 +26,147 @@ import {
} from '../types';
import { sanitizeHtml } from './series';
+/**
+ * Escapes RegExp metacharacters in a string so it can be safely used in a
+ * dynamically created regular expression.
+ *
+ * @param value - The raw string to escape
+ * @returns The escaped string safe for use in `new RegExp(...)`
+ */
+const escapeRegex = (value: string) =>
+ value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
+
+/**
+ * Replaces a label inside a compound key only if it appears as a complete
+ * word/token and contains at least one alphabetic character.
+ *
+ * @param key - The source string (typically a compound field name)
+ * @param label - The label to search for as a standalone token
+ * @param replacement - The human-readable value to replace the label with
+ * @returns The transformed key if a valid match exists, otherwise the
original key
+ */
+const replaceLabelIfExists = (
+ key: string,
+ label: string,
+ replacement: string,
+) => {
+ /**
+ * Logic:
+ *
+ * This function is intentionally stricter than a simple substring replace:
+ * - The label must NOT be part of a larger word (e.g. "account" will NOT
match
+ * "testing_account").
+ * - Underscores (`_`) are treated as part of the word.
+ * - Numeric-only matches are ignored (e.g. "12" will NOT match "123").
+ *
+ * If the label is found, only the matched portion is replaced; otherwise,
+ * the original key is returned unchanged.
+ *
+ * Examples:
+ * - replaceLabelIfExists("testing_account 123", "testing_account",
"Account")
+ * → "Account 123"
+ * - replaceLabelIfExists("testing_account 123", "account", "Account")
+ * → "testing_account 123"
+ * - replaceLabelIfExists("123", "12", "X")
+ * → "123"
+ */
+
+ if (key === label) {
+ return replacement;
+ }
+
+ const escapedLabel = escapeRegex(label);
+ const regex = new RegExp(`(?<!\\w)${escapedLabel}(?!\\w)`, 'g');
+ return regex.test(key) ? key.replace(regex, replacement) : key;
Review Comment:
**Suggestion:** The documentation states numeric-only matches are ignored,
but the implementation has no alphabetic check and will replace exact numeric
labels directly; this creates behavior that contradicts the documented matching
contract and can unexpectedly rename numeric tokens. Add an explicit
non-numeric guard (or correct the docs if numeric replacement is intended).
[docstring mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Documentation misstates behavior for numeric-only label inputs.
- ⚠️ Callers may rely on non-existent numeric ignore guarantee.
- ⚠️ Could cause unexpected renaming of numeric tokens.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. In
`superset-frontend/plugins/plugin-chart-echarts/src/utils/forecast.ts:53-72`,
the
comment for `replaceLabelIfExists` states that numeric-only matches are
ignored, but the
implementation at 74-80 performs replacements whenever `key === label` or
the regex
matches.
2. Import `replaceLabelIfExists` into a unit test and invoke it with `key =
'12'`, `label
= '12'`, and `replacement = 'X'`.
3. The equality check at line 74 sees `key === label` and returns `'X'`,
demonstrating
that numeric-only labels are replaced despite the comment claiming they are
ignored.
4. Similarly, if a numeric label such as `'12'` were ever included in
`newLabelMap` by
`addLabelMapToVerboseMap` at 132-142, the regex defined at 78-79 would
permit replacements
of standalone numeric tokens, which again conflicts with the documented
matching contract
and could surprise future callers relying on the comment.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=43b1bfb50be049afaba3f00a989cf5d5&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=43b1bfb50be049afaba3f00a989cf5d5&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/plugins/plugin-chart-echarts/src/utils/forecast.ts
**Line:** 74:80
**Comment:**
*Docstring Mismatch: The documentation states numeric-only matches are
ignored, but the implementation has no alphabetic check and will replace exact
numeric labels directly; this creates behavior that contradicts the documented
matching contract and can unexpectedly rename numeric tokens. Add an explicit
non-numeric guard (or correct the docs if numeric replacement is intended).
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37229&comment_hash=31798dd5675d4c2d75b7501c04a409b968230408a19a6c3ccbb4e3e4c1d488a9&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37229&comment_hash=31798dd5675d4c2d75b7501c04a409b968230408a19a6c3ccbb4e3e4c1d488a9&reaction=dislike'>👎</a>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
