codeant-ai-for-open-source[bot] commented on code in PR #43303:
URL: https://github.com/apache/superset/pull/43303#discussion_r3805053428
##########
superset-frontend/scripts/gen-qc-registry.mjs:
##########
@@ -0,0 +1,187 @@
+// Deterministic, re-runnable codegen: maps every plugin `buildQuery` module
to the
+// viz_type key(s) it is registered under, and emits registry.generated.ts
consumed
+// by entry.ts. Join: buildQuery module <- (index.ts that imports it) ->
plugin class
+// name -> MainPreset `.configure({ key: VizType.X })` -> VizType enum string.
+// A single builder legitimately maps to several keys (e.g. echarts_timeseries
+ _line/_bar/...).
+import { readFileSync, writeFileSync, readdirSync, statSync, existsSync } from
'node:fs';
+import path from 'node:path';
+import { fileURLToPath } from 'node:url';
+
+const FE = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..');
+const PLUGINS = path.join(FE, 'plugins');
+const OUT = path.join(FE, 'src', 'backend-querycontext',
'registry.generated.ts');
+
+// --- 1. VizType enum: EnumName -> 'string_value' ---
+const vizTypeSrc = readFileSync(
+ path.join(FE, 'packages/superset-ui-core/src/chart/types/VizType.ts'),
+ 'utf8',
+);
+const VIZ_ENUM = {};
+for (const m of vizTypeSrc.matchAll(/(\w+)\s*=\s*['"]([\w-]+)['"]/g))
VIZ_ENUM[m[1]] = m[2];
+
+// --- 2. MainPreset: ClassName -> viz string ---
+const mainPreset = readFileSync(
+ path.join(FE, 'src/visualizations/presets/MainPreset.js'),
+ 'utf8',
+);
+// MainPreset renames on import (e.g. `import { PivotTableChartPlugin as
+// PivotTableChartPluginV2 } from '...'`), then `new
PivotTableChartPluginV2()`. Map
+// each local `new X()` name back to the ORIGINAL package export name the
codegen sees.
+const importOrig = {}; // localName -> package-export name
+for (const im of
mainPreset.matchAll(/import\s*\{([^}]*)\}\s*from\s*['"][^'"]+['"]/g)) {
+ for (let spec of im[1].split(',')) {
+ spec = spec.trim();
+ if (!spec) continue;
+ const as = spec.match(/^(\w+)\s+as\s+(\w+)$/);
+ if (as) importOrig[as[2]] = as[1];
+ else if (/^\w+$/.test(spec)) importOrig[spec] = spec;
+ }
+}
+const CLASS_TO_VIZ = {};
+for (const m of mainPreset.matchAll(
+ /new\s+(\w+)\s*\(\s*\)\s*\.configure\(\s*\{\s*key:\s*VizType\.(\w+)/g,
Review Comment:
**Suggestion:** The registration matcher only accepts plugins instantiated
with an empty constructor. `CartodiagramPlugin` is registered as `new
CartodiagramPlugin({ ... }).configure(...)`, so its otherwise discoverable
`buildQuery` is placed in `unmapped` and `entry.ts` returns `__unsupported__`
for cartodiagram charts, silently falling back to the less faithful generic
derivation. Match constructor arguments or use a parser/executable registration
source instead of requiring `()`. [incomplete implementation]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Cartodiagram imports miss frontend query-context generation.
- ⚠️ Cartodiagram-specific query behavior is replaced by generic derivation.
- ⚠️ Unsupported fallback is logged only at debug level.
```
</details>
[](https://github.com/CodeAnt-AI/skills/blob/main/skills/codeant-resolve-pr-comments/SKILL.md)
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/scripts/gen-qc-registry.mjs
**Line:** 42:42
**Comment:**
*Incomplete Implementation: The registration matcher only accepts
plugins instantiated with an empty constructor. `CartodiagramPlugin` is
registered as `new CartodiagramPlugin({ ... }).configure(...)`, so its
otherwise discoverable `buildQuery` is placed in `unmapped` and `entry.ts`
returns `__unsupported__` for cartodiagram charts, silently falling back to the
less faithful generic derivation. Match constructor arguments or use a
parser/executable registration source instead of requiring `()`.
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%2F43303&comment_hash=bef12757a86479b8d8bd117520138c66bf9ef0052608e178424f2f6af256a8fd&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43303&comment_hash=bef12757a86479b8d8bd117520138c66bf9ef0052608e178424f2f6af256a8fd&reaction=dislike'>👎</a>
##########
superset-frontend/src/backend-querycontext/parity.test.ts:
##########
@@ -0,0 +1,75 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/**
+ * Parity golden-writer (Apache Superset #33615).
+ *
+ * For each shared input fixture in `__fixtures__/formdata/<viz>.json`, run the
+ * exact backend code path (`generateQueryContext`) and record the resulting
+ * query_context to `__fixtures__/expected/<viz>.json`. The Python parity test
+ *
(`tests/unit_tests/charts/commands/importers/v1/query_context_parity_test.py`)
+ * then runs the SAME `generateQueryContext` in V8 over the SAME inputs and
+ * asserts byte-equality with these goldens — proving the backend synthesis is
+ * pixel-faithful to the frontend `buildQuery`.
+ *
+ * FOUNDATION: 2 viz types today (pivot_table_v2, echarts_timeseries) — the
+ * cases where the generic Python derivation diverges. Full "perfect" coverage
=
+ * a fixture per registered viz type (follow-on, ideally code-generated).
+ */
+import fs from 'fs';
+import path from 'path';
+import { generateQueryContext, VIZ_TYPES } from './entry';
+
+const FORMDATA_DIR = path.join(__dirname, '__fixtures__', 'formdata');
+const EXPECTED_DIR = path.join(__dirname, '__fixtures__', 'expected');
+
+describe('backend query-context parity goldens', () => {
+ beforeAll(() => {
+ fs.mkdirSync(EXPECTED_DIR, { recursive: true });
+ });
+
+ const fixtures = fs
+ .readdirSync(FORMDATA_DIR)
+ .filter(f => f.endsWith('.json'))
+ .map(f => f.replace(/\.json$/, ''));
+
+ it.each(fixtures)('records + validates query_context for %s', vizType => {
+ const formData = JSON.parse(
+ fs.readFileSync(path.join(FORMDATA_DIR, `${vizType}.json`), 'utf8'),
+ );
+ const raw = generateQueryContext(vizType, JSON.stringify(formData));
+ const result = JSON.parse(raw);
Review Comment:
**Suggestion:** This test creates its expected output by invoking the same
`generateQueryContext` implementation that the backend uses, then writes that
output as the golden. Consequently, a wrong `buildQuery` mapping or incorrect
query-context implementation can produce matching goldens and leave the test
green; it only checks serialization/runtime consistency, not frontend/backend
parity. The expected fixtures need to be produced by an independent frontend
execution or independently maintained expected values. [incomplete
implementation]
<details>
<summary><b>Severity Level:</b> Minor 🧹</summary>
```mdx
- ⚠️ Parity CI can miss incorrect registry mappings.
- ⚠️ Frontend/backend query-context divergence may reach imports.
- ⚠️ Golden files provide consistency, not independent correctness.
```
</details>
[](https://github.com/CodeAnt-AI/skills/blob/main/skills/codeant-resolve-pr-comments/SKILL.md)
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/backend-querycontext/parity.test.ts
**Line:** 56:57
**Comment:**
*Incomplete Implementation: This test creates its expected output by
invoking the same `generateQueryContext` implementation that the backend uses,
then writes that output as the golden. Consequently, a wrong `buildQuery`
mapping or incorrect query-context implementation can produce matching goldens
and leave the test green; it only checks serialization/runtime consistency, not
frontend/backend parity. The expected fixtures need to be produced by an
independent frontend execution or independently maintained expected values.
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%2F43303&comment_hash=cf441abf89db6c146ca0319e4e6f6b64022e6169749131ece451270369a06d22&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43303&comment_hash=cf441abf89db6c146ca0319e4e6f6b64022e6169749131ece451270369a06d22&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]
