release: v0.9.1 — Stability + App Features

[alfredo1996]

Summary

Merges release/0.9.1 into dev with all stability and feature work for v0.9.1.

Stability (#193–#198, #204)

• Connection pluggability: abstract driver type, wrapError normalization, split AdvancedConnectionOptions
• Coverage push: app hooks 18%→61%, component 77%→85%, cypher-lang smoke tests
• E2E tests for v0.9 features, flaky tests marked test.fixme()
• CI + CodeRabbit config: added release/* branches

App Features (#188)

• CSV export for widget cards (RFC 4180, CRLF, formula-prefix quoting)
• GFM markdown tables with alignment markers
• Missing parameter badges with scroll-to-source
• Data transforms pipeline: filter, sort, groupBy, calculatedColumn, rename, limit
  • Dedicated Transform tab in widget editor
  • Parameter support ($param_xxx) in filter values and expressions
  • 48 unit tests

Security fixes

• XSS: split isSafeUrl into link/image validators, strip tab/newline bypass
• CSV injection: quote cells starting with =, @, +, -

Code quality

• Zustand store refactor: eliminated bidirectional sync in widget-editor-modal
• All CodeRabbit findings addressed
• Null guard in matchesFilter, transform validation, graph chart skip

Test plan

• 1467 app unit tests passing
• 1198 component unit tests passing
• 208 E2E tests passing (5 skipped — known flakes)
• TypeScript type-check passing
• Build passing

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Visual Transform editor and client-side data transforms; CSV export with smart filenames; parameter source mapping and smooth scroll+highlight for widgets.

• Bug Fixes

  • More consistent query caching/editor save behavior and standardized connector error handling.

• Improvements

  • Improved Markdown table rendering and stricter link/image URL validation; CI/review triggers now include release branches; chart settings support a Transform tab.

• Tests

  • Many new/updated Vitest and E2E suites; test config split for unit vs component projects.

• Chores

  • ESLint and dev-dependency/test setup updates.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

Walkthrough

Adds a client-side transforms DSL and editor, CSV/PNG export utilities, parameter-source mapping with scroll-to-widget highlighting/navigation, Card/Dashboard prop changes, many new Vitest/jsdom tests and setup, connection package type/error refactors, and CI/review trigger updates for release branches.

Changes

Cohort / File(s)	Summary
Config & CI /.coderabbit.yaml, .github/workflows/ci.yml, eslint.config.js, package.json	Expand review/CI triggers to release/*; add @next/eslint-plugin-next and integrate Next.js lint rules; update devDependencies.
Vitest & Test Setup app/vitest.config.ts, app/vitest.setup.tsx, app/package.json, CLAUDE.md	Introduce multi-project Vitest config (unit node, component jsdom), jsdom setup with Next.js mocks, and testing lib deps and guidance.
E2E Tests app/e2e/... (charts.spec.ts, widget-lab.spec.ts, widgets.spec.ts, transforms.spec.ts)	Reformat Playwright tests; mark several flaky cases test.fixme(...); add new transforms E2E stubs.
Dashboard pages & routing app/src/app/(dashboard)/[id]/page.tsx, .../edit/page.tsx	Compute/pass parameterSourceMap; extend navigate handler to accept optional scrollToWidgetId and call scrollToWidgetWhenReady; change preview cache lookup/invalidation keys.
Card & Dashboard components app/src/components/card-container.tsx, app/src/components/dashboard-container.tsx	Update onNavigateToPage signature to accept optional scroll target; add parameterSourceMap prop; add MissingParamBadge; wire export CSV to read cached queries and apply transforms; refactor styling and column extraction helpers.
Widget editor & store app/src/components/widget-editor-modal.tsx, app/src/components/widget-editor/transform-editor.tsx, app/src/stores/widget-editor-store.ts	Move editor fields into store; add transforms state and setter; add TransformEditor component and persist transforms into widget.settings.
Transforms library & tests app/src/lib/data-transforms.ts, app/src/lib/__tests__/data-transforms.test.ts	Add typed Transform DSL and applyTransforms(data, transforms, paramValues?) implementing filter/sort/group/agg/calculated/rename/limit with comprehensive tests and param resolution.
Parameter discovery & scroll helpers app/src/lib/collect-parameter-names.ts, app/src/lib/scroll-to-widget.ts, app/src/lib/__tests__/*	Add buildParameterSourceMap types/implementation and scrollAndHighlight/scrollToWidgetWhenReady helpers with tests; dedupe and aggregate parameter sources across pages.
CSS utilities app/src/app/globals.css	Add @keyframes widget-highlight-pulse and .widget-highlight utility for highlight animation.
Card utils & export app/src/lib/card-utils.ts, component/src/lib/export-utils.ts, component/src/lib/__tests__/export-utils.test.ts, component/src/utils/index.ts	Add extract/resolve/build-export helpers and CSV/filename/download utilities with tests and re-exports.
Widget actions & utils app/src/lib/widget-actions.ts, app/src/lib/widget-utils.ts, app/src/lib/__tests__/*	Add helpers to build click-action/styling configs, classify data widgets, get display titles, and template staleness checks with tests.
Chart settings & markdown component/src/components/composed/chart-settings-panel.tsx, component/src/components/composed/markdown-widget.tsx, component/src/components/composed/__tests__/markdown-tables.test.tsx	Add optional transform tab prop; tighten link/image URL validation; improve GFM table parsing/alignment; add table tests.
Query executor minor app/src/lib/query-executor.ts	Rename local cached var module → connModule and update usages (no behavior change).
Connection package refactor & errors connection/src/generalized/interfaces.ts, connection/src/generalized/AuthenticationModule.ts, connection/src/index.ts, connection/src/neo4j/*, connection/src/postgresql/*, tests	Split AdvancedConnectionOptions into Neo4jAdvancedOptions/PostgresAdvancedOptions; narrow constructor arg types; centralize error wrapping to ConnectorError/ConnectorErrorType; add re-exports; update tests to expect ConnectorError.
Many new tests app/src/hooks/__tests__/*, app/src/lib/__tests__/*, component/src/lib/cypher-lang/__tests__/*, component/src/components/..., component/src/lib/__tests__/*	Add numerous Vitest suites covering hooks, transforms, card utils, widget actions, markdown tables, cypher-lang, TransformEditor, scroll-to-widget, and more.
Formatting & quoting various connection tests and files	Consistent quoting/formatting updates and minor refactors across connection modules and tests.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant UI as Widget Editor
    participant Store as EditorStore
    participant API as Backend API
    participant DB as Dashboard Storage
    rect rgba(200,220,255,0.5)
    User->>UI: Open editor
    UI->>Store: loadFromWidget(widget)
    Store->>Store: hydrate transforms
    User->>UI: Edit transforms
    UI->>Store: setTransforms(...)
    User->>UI: Save widget
    UI->>API: POST /api/dashboards/{id}/widgets (includes settings.transforms)
    API->>DB: Persist widget
    DB-->>API: Success
    API-->>UI: 200 OK
    end

Loading

sequenceDiagram
    participant User
    participant Menu as Widget Menu
    participant Cache as ReactQuery Cache
    participant Transform as applyTransforms
    participant Export as Export Utils
    rect rgba(200,255,220,0.5)
    User->>Menu: Click "Export CSV"
    Menu->>Cache: getQueriesData(["widget-query", connId, query])
    Cache-->>Menu: cached results
    Menu->>Transform: applyTransforms(data, transforms, paramValues)
    Transform-->>Menu: transformedRows
    Menu->>Export: buildCsvString(transformedRows)
    Export-->>Menu: csvContent
    Menu->>Export: buildExportFilename(title,"csv",dashboardName)
    Export-->>Menu: filename.csv
    Menu->>Export: triggerDownload(csvContent, filename)
    Export->>User: Browser download
    end

Loading

sequenceDiagram
    participant User
    participant Card as CardContainer
    participant ParamStore as Parameter Store
    participant Dashboard as DashboardContainer
    participant Scroll as scrollToWidgetWhenReady
    rect rgba(255,230,200,0.5)
    User->>Card: Click widget (set-parameter)
    Card->>ParamStore: setParameter(name,value)
    User->>Card: Navigate to widget source
    Card->>Dashboard: onNavigateToPage(pageId, scrollToWidgetId)
    Dashboard->>Scroll: scrollToWidgetWhenReady(scrollToWidgetId)
    Scroll-->>Dashboard: success/failure
    end

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

• feat: app features — CSV export, GFM tables, param badges, data transforms #188: Overlaps transforms, CSV export, parameter-source mapping, scroll-to-widget, and editor/container prop changes.
• release: v0.9 — Table Features, Chart Improvements, Gauge Thresholds #199: Related dashboard/widget changes (transform editor, parameter mapping, scroll-to-widget) with overlapping code paths.
• fix(app): address CodeRabbit review findings from PR #82 #83: Modifies widget-editor-modal and touches transform/editor wiring; likely to conflict or interact.

Suggested labels

enhancement, pkg:app, pkg:component, pkg:connection, testing, area:widgets, chore

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 60.56% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main release objective with version number and key themes (stability + app features) matching the substantial changeset.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch release/0.9.1

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 11

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (6)

app/e2e/widgets.spec.ts (1)

79-94: ⚠️ Potential issue | 🟠 Major

The JSON viewer E2E is still flaky/failing in CI; add a sync point before “Add Widget”.

This test doesn’t synchronize on query execution/preview readiness before submit, which can race in CI. Add the same run+preview gate used in other widget tests.

Suggested hardening

     await typeInEditor(dialog, page, "MATCH (m:Movie) RETURN m LIMIT 3");

     await expect(
       dialog.getByRole("button", { name: "Add Widget" }),
     ).toBeEnabled({ timeout: 10_000 });
+    await dialog.getByTitle("Run query (Ctrl+Enter / ⌘+Enter)").click();
+    await expect(getPreview(dialog)).toBeVisible({ timeout: 15_000 });
     await dialog.getByRole("button", { name: "Add Widget" }).click();
+    await expect(dialog).not.toBeVisible({ timeout: 10_000 });

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/e2e/widgets.spec.ts` around lines 79 - 94, The test "should add a JSON
viewer widget" races because it submits before the query preview finishes; after
typeInEditor(dialog, page, "...") use the same run+preview synchronization as
other widget tests: click the dialog's "Run" button (dialog.getByRole("button",
{ name: "Run" })) and wait for the preview to be ready (e.g., wait for the
preview result/table/preview-region locator or for any loading spinner to
disappear) before asserting the "Add Widget" button enabled and clicking
dialog.getByRole("button", { name: "Add Widget" }); this ensures the query
execution/preview gate is satisfied prior to submit.

app/e2e/widget-lab.spec.ts (1)

98-134: ⚠️ Potential issue | 🟠 Major

Stabilize the save-template flow; this test is currently red in CI.

The failing test still depends on eventual UI/list timing after save. Capture and assert the create API response directly, then set templateId from that response to remove race conditions.

Suggested stabilization

-      // Save
-      await saveDialog.getByRole("button", { name: "Save Template" }).click();
-      await expect(saveDialog).not.toBeVisible();
+      // Save and wait for backend confirmation (prevents CI race)
+      const createTemplateResponse = page.waitForResponse(
+        (res) =>
+          res.request().method() === "POST" &&
+          res.url().includes("/api/widget-templates"),
+      );
+      await saveDialog.getByRole("button", { name: "Save Template" }).click();
+      const createTemplate = await createTemplateResponse;
+      expect(createTemplate.ok()).toBeTruthy();
+      const created = (await createTemplate.json()).data as { id: string };
+      templateId = created.id;
+      await expect(saveDialog).not.toBeVisible();

       // Navigate to Widget Lab and verify the template appears
       await page.goto("/widget-lab");
       await expect(page.getByText(templateName)).toBeVisible({
         timeout: 10_000,
       });

-      // Capture template id for cleanup
-      const res = await page.request.get("/api/widget-templates");
-      const templates = (await res.json()).data;
-      // eslint-disable-next-line `@typescript-eslint/no-explicit-any`
-      const saved = templates.find((t: any) => t.name === templateName);
-      templateId = saved?.id;
+      expect(templateId).toBeTruthy();

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/e2e/widget-lab.spec.ts` around lines 98 - 134, The test currently races
waiting for the template to appear in the UI; instead, after filling saveDialog
and before asserting invisibility, wait for and capture the backend create
response (e.g. use page.waitForResponse or page.waitForRequest to match POST
/api/widget-templates), assert the response status and parse the JSON to extract
the created template id, assign it to templateId, and then continue with UI
assertions (remove the later GET /api/widget-templates and templates.find
logic). Target symbols: the test block named "can save a widget as a template
and see it in Widget Lab", the saveDialog interaction code, the templateName
variable, and the templateId assignment.

connection/src/postgresql/PostgresAuthenticationModule.ts (1)

124-127: ⚠️ Potential issue | 🟡 Minor

Re-validate URI in updateAuthConfig for consistency.

Line [125] checks only presence/auth fields. It should also enforce the same URI protocol/format guard used in the constructor, otherwise invalid URIs can be accepted until later runtime failure.

Suggested patch

 async updateAuthConfig(authConfig: AuthConfig): Promise<void> {
   this._checkConfigurationConsistency(authConfig);
+  this._validateUri(authConfig.uri, ["postgresql:", "postgres:"]);
   this._authConfig = authConfig;

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@connection/src/postgresql/PostgresAuthenticationModule.ts` around lines 124 -
127, updateAuthConfig currently only calls _checkConfigurationConsistency and
sets this._authConfig, but does not re-validate the connection URI format that
the constructor enforces, so invalid URIs can slip in; modify updateAuthConfig
to run the same URI/protocol validation used in the constructor (reuse or call
the constructor's URI validation helper or extract it into a shared method)
before assigning this._authConfig, ensuring the URI/protocol guard (the same
checks used when constructing PostgresAuthenticationModule) is applied and
throw/raise the same error if the URI is invalid.

connection/__tests__/connection/query-write.ts (1)

12-17: ⚠️ Potential issue | 🟠 Major

afterAll only closes one of the two drivers here.

The first test's Neo4jConnectionModule is overwritten by the second assignment to connection, so only the last driver gets closed. Close each driver in afterEach, or create/close inside each test, otherwise this suite can still leak handles and hang workers.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@connection/__tests__/connection/query-write.ts` around lines 12 - 17, The
test suite currently only closes the last assigned Neo4j driver because the
shared variable connection gets overwritten; change the teardown so each driver
is closed: either create and close the Neo4jConnectionModule (or its driver via
getDriver().close()) inside each test, or replace the single afterAll() with an
afterEach() that checks the test-scoped connection (the connection variable or a
locally stored array of created connections) and calls await
connection.getDriver().close() for each created instance; ensure you reference
the existing Neo4jConnectionModule/connection and getDriver().close() calls so
no driver instance is left open.

connection/__tests__/connection/query-basic.ts (1)

20-27: ⚠️ Potential issue | 🟠 Major

Fail these tests from onFail, not just console.error.

If runQuery reports failures through onFail without rejecting, both tests still pass and hide real regressions. Re-throw the error here.

Suggested fix

       onFail: (err) => {
         console.error("Error executing query:", err);
+        throw err;
       },

Also applies to: 45-52

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@connection/__tests__/connection/query-basic.ts` around lines 20 - 27, The
test's QueryCallback implementation (queryCallback) currently swallows failures
by only logging in onFail; update the onFail handler used by runQuery to
re-throw the received error (or call the test's fail assertion) instead of just
console.error so the test fails when onFail is invoked; ensure you change both
occurrences of the QueryCallback onFail (the one around lines shown and the
second occurrence) so any invocation by runQuery surfaces as a test failure.

app/src/components/dashboard-container.tsx (1)

135-140: ⚠️ Potential issue | 🟡 Minor

Missing CSS.escape for widget ID selector.

This function constructs a selector without escaping, while scroll-to-widget.ts correctly uses CSS.escape. Widget IDs with special characters (e.g., colons) will fail to match.

Proposed fix

   const scrollToSource = useCallback((sourceWidgetId?: string) => {
     if (!sourceWidgetId) return;
     document
-      .querySelector(`[data-widget-id="${sourceWidgetId}"]`)
+      .querySelector(`[data-widget-id="${CSS.escape(sourceWidgetId)}"]`)
       ?.scrollIntoView({ behavior: "smooth", block: "center" });
   }, []);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/src/components/dashboard-container.tsx` around lines 135 - 140, The
selector in scrollToSource does not escape widget IDs, so IDs with special
characters won't match; update the useCallback scrollToSource (in
dashboard-container.tsx) to call CSS.escape on sourceWidgetId before building
the attribute selector (mirroring scroll-to-widget.ts) and then use
document.querySelector(`[data-widget-id="${escapedId}"]`)?.scrollIntoView(...);
ensure you handle the case where CSS.escape may be undefined in older
environments by still calling it (native browsers support it) and keep the early
return when sourceWidgetId is falsy.

🧹 Nitpick comments (8)

component/src/lib/cypher-lang/__tests__/cypher-lang-smoke.test.ts (1)

77-83: Prefer stable key assertions over map-size threshold.

Object.keys(...).length >= 20 is brittle for smoke tests and can fail on harmless internal reshuffles. Checking explicit required keys is more stable.

♻️ Suggested change

   it("tokenTypeToStyleTag has entries for all highlighted types", async () => {
     const { tokenTypeToStyleTag } = await import("../constants");
-    expect(Object.keys(tokenTypeToStyleTag).length).toBeGreaterThanOrEqual(20);
     expect(tokenTypeToStyleTag.keyword).toBeDefined();
     expect(tokenTypeToStyleTag.comment).toBeDefined();
     expect(tokenTypeToStyleTag.stringLiteral).toBeDefined();
+    expect(tokenTypeToStyleTag.numberLiteral).toBeDefined();
+    expect(tokenTypeToStyleTag.operator).toBeDefined();
   });

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@component/src/lib/cypher-lang/__tests__/cypher-lang-smoke.test.ts` around
lines 77 - 83, Replace the brittle map-size assertion in the test for
tokenTypeToStyleTag with explicit checks for required keys: remove the
Object.keys(tokenTypeToStyleTag).length >= 20 assertion and instead assert that
specific entries exist on tokenTypeToStyleTag (e.g.,
tokenTypeToStyleTag.keyword, tokenTypeToStyleTag.comment,
tokenTypeToStyleTag.stringLiteral and any other essential types such as number,
operator, variable or function used elsewhere). Update the test in
cypher-lang-smoke.test.ts to only assert existence of these named properties
rather than relying on a threshold count.

app/e2e/charts.spec.ts (1)

362-365: Track each test.fixme with issue ID and exit criteria.

Line 362 and Line 996 skip important chart behaviors; please add a linked issue and a short unblock condition in the adjacent comment so these don’t become permanent blind spots.

Also applies to: 995-997

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/e2e/charts.spec.ts` around lines 362 - 365, The two test.skip markers
using test.fixme (notably the test labeled "PostgreSQL pie chart — fetches data
and renders canvas" and the other test.fixme around lines 995-997) need a short
adjacent comment that cites an issue ID and clear exit criteria; update each
test.fixme by adding a comment directly above it that includes the tracking
issue (e.g., ISSUE-1234) and concise unblock conditions (what must be fixed to
re-enable the test, e.g., "CM6 __cmView race fixed and typeInEditor stable in
CI"), so reviewers can trace and close the blind spot and know when to re-enable
the test.

connection/src/generalized/interfaces.ts (1)

244-247: Add a discriminator field to AdvancedConnectionOptions union for type-safe narrowing.

The union of Neo4jAdvancedOptions and PostgresAdvancedOptions has no required discriminator field, and all fields are optional in both types. While the factory pattern avoids this issue by dispatching via ConnectionTypes enum, code handling the union directly cannot reliably distinguish between the two. Add a required connectorType field to both option interfaces to enable proper TypeScript union narrowing.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@connection/src/generalized/interfaces.ts` around lines 244 - 247, Add a
required discriminator field to the advanced options so the union
AdvancedConnectionOptions can be type-narrowed: add a required connectorType:
ConnectionTypes.NEO4J to the Neo4jAdvancedOptions interface and connectorType:
ConnectionTypes.POSTGRES to the PostgresAdvancedOptions interface (or string
literals matching the existing ConnectionTypes enum), then rebuild the
AdvancedConnectionOptions union; update any creation code that constructs these
option objects to include the new connectorType property.

app/src/components/widget-editor/transform-editor.tsx (3)

219-302: groupBy UI supports only single aggregation.

The data model allows multiple aggregations but the UI only shows/edits the first. Consider documenting this limitation or expanding the UI in a future iteration.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/src/components/widget-editor/transform-editor.tsx` around lines 219 -
302, The GroupBy UI only reads/edits the first aggregation in
transform.aggregations, so multiple aggregations are ignored; update the UI to
support multiple aggregations (or explicitly document the limitation). Fix by
changing the Group Column/Aggregate/Function blocks (the Selects that read/write
transform.aggregations[0]) to map over transform.aggregations and render a row
per aggregation with controls bound to each aggregation (using index or id) and
call onChange with the updated aggregations array, and include an "Add
aggregation" and "Remove" action to push/splice entries; ensure AGG_FUNCTIONS is
used for each aggregation's function Select and keep transform.type checks
intact.

---

143-148: Overly complex type inference for operator.

The conditional type extraction is verbose. A simpler union cast would suffice.

Simpler alternative

                 onValueChange={(v) =>
                   onChange({
                     ...transform,
-                    operator: v as Transform & { type: "filter" } extends {
-                      operator: infer O;
-                    }
-                      ? O
-                      : never,
+                    operator: v as typeof FILTER_OPERATORS[number]["value"],
                   })
                 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/src/components/widget-editor/transform-editor.tsx` around lines 143 -
148, The code uses an overly verbose conditional type to extract operator from
v; simplify by casting v to the specific variant instead of the conditional
inference — for example, treat v as Transform & { type: "filter" } (or use
Extract<Transform, { type: "filter" }>) and use that cast to get the operator
type for the operator field; update the code around the operator assignment in
transform-editor.tsx (the v variable and Transform type usage) to use this
simpler cast-based extraction.

---

414-424: Using array index as React key is fragile.

When transforms are removed from the middle, React may incorrectly reconcile components. Consider adding a stable id field to each transform.

Suggested approach

Add a unique ID when creating transforms:

 function makeDefault(type: string, columns: string[]): Transform {
   const col = columns[0] ?? "";
+  const id = crypto.randomUUID();
   switch (type) {
     case "filter":
-      return { type: "filter", column: col, operator: "==", value: "" };
+      return { id, type: "filter", column: col, operator: "==", value: "" };
     // ... etc for other types

Then use key={t.id} instead of key={i}.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/src/components/widget-editor/transform-editor.tsx` around lines 414 -
424, The list rendering uses the array index as the React key (in transforms.map
-> TransformCard key={i}), which can break reconciliation when items are
inserted/removed; modify the transform data model to include a stable unique id
(e.g., add an id when creating new transforms in whatever function constructs
them) and update rendering to use that stable id (key={t.id}); ensure any
functions that update or remove transforms (updateTransform, removeTransform)
carry or locate transforms by this id rather than array index so state updates
remain correct.

app/src/components/widget-editor-modal.tsx (1)

164-174: Redundant store initialization with existing reset effect.

This effect initializes the store, but the larger effect starting at line 429 also sets all state fields explicitly (including store setters). Both effects run on modal open, causing redundant updates.

Consider removing the duplicate logic from the effect at line 429+ now that the store handles initialization via loadFromWidget/resetForAdd.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/src/components/widget-editor-modal.tsx` around lines 164 - 174, Remove
the redundant initialization: the useEffect that calls
useWidgetEditorStore.getState().loadFromWidget(widget) / resetForAdd() on modal
open duplicates the later effect that sets all store fields explicitly, so
delete this early effect and rely on the single effect that performs full state
initialization (the effect that sets all state fields and store setters) to
avoid double updates; ensure the remaining effect uses
loadFromWidget/resetForAdd as appropriate or directly sets the store
consistently via useWidgetEditorStore methods (loadFromWidget, resetForAdd) so
there are no conflicting initializations.

component/src/lib/__tests__/export-utils.test.ts (1)

9-61: Add CSV-injection regression cases here.

This suite never asserts the hardening for cells starting with =, +, -, or @. Since this PR treats those prefixes as unsafe, they need explicit tests so the fix cannot regress silently.

Suggested test cases

 describe("escapeCsvCell", () => {
+  it("quotes formula-prefixed cells to prevent CSV injection", () => {
+    expect(escapeCsvCell("=1+1")).toBe('"=1+1"');
+    expect(escapeCsvCell("+SUM(A1:A2)")).toBe('"+SUM(A1:A2)"');
+    expect(escapeCsvCell("-10")).toBe('"-10"');
+    expect(escapeCsvCell("@cmd")).toBe('"@cmd"');
+  });
+
   it("returns empty string for null", () => {
     expect(escapeCsvCell(null)).toBe("");
   });

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@component/src/lib/__tests__/export-utils.test.ts` around lines 9 - 61, Add
regression tests to export-utils.test.ts covering CSV-injection hardening for
values that start with the unsafe prefixes "=", "+", "-", and "@" so the
escapeCsvCell behavior cannot regress; specifically, add test cases calling
escapeCsvCell with inputs beginning with each of those characters (including
variants like a leading quote or whitespace if relevant) and assert the returned
CSV cell is the hardened form produced by escapeCsvCell (use the same expected
output pattern as other tests), referencing the escapeCsvCell function to locate
where to add them.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@app/src/app/`(dashboard)/[id]/edit/page.tsx:
- Around line 321-329: The preview uses queryClient.getQueriesData with a
partial key ["widget-query", widget.connectionId, widget.query], which can
return nondeterministic entries when the same widget/query has cached results
for different parameter states; update openEditWidget to include the current
parameter state in the cache lookup (e.g., add widget.params or the current
parameter values to the queryKey) so you retrieve the exact cached entry for the
active parameters, or if you intentionally need to match multiple entries, pick
the most recent matching entry (by comparing timestamps/metadata on the returned
entries) instead of taking entries[0]; apply the same fix in exportWidgetCsv to
ensure it looks up the cache with the full parameter-aware key (or chooses the
most recent matching entry) so both preview and CSV export use consistent,
parameter-specific cached data.

In `@app/src/components/card-container.tsx`:
- Around line 558-565: The live-data branch is still sending graph widget
payloads through applyTransforms, causing non-tabular mappedData to be
transformed incorrectly; update the transformedData assignment so transforms are
applied only when dataTransforms exist AND the mappedData is tabular (e.g.,
Array.isArray(mappedData) and items are plain objects) or when the chartConfig
explicitly indicates a non-graph widget (use an existing flag like
chartConfig.isGraph if present); change the conditional that builds
transformedData (the code that calls chartConfig.transformWithMapping,
mappedData, applyTransforms, dataTransforms, and allParamValues) to skip
applyTransforms for graph/non-tabular mappedData and return mappedData directly
in that case.

In `@app/src/components/widget-editor/transform-editor.tsx`:
- Around line 332-353: The current parsing in the Input onChange for
transform.type === "renameColumns" (reading transform.mapping) naively splits on
commas and '=' which breaks for column names containing those characters; update
the handler for the Input change (the onChange that builds mapping from
e.target.value) to use a robust parse strategy — either require and parse a JSON
object string or implement a small CSV/key-value parser that supports quoted
values and escaping (respecting commas and equals inside quotes) — then set
onChange({ ...transform, mapping }) with the reliably parsed
Record<string,string>; keep references to transform.mapping, the Input onChange,
and the mapping construction so reviewers can find the change.

In `@app/src/hooks/__tests__/use-seed-query.test.ts`:
- Around line 196-201: Replace the untyped "as any" cast by giving the mock
function a proper TypeScript signature: declare the mock implementation
parameter using Parameters<typeof useQuery>[0] and its return type using
ReturnType<typeof useQuery>, e.g. implement the mock as (config:
Parameters<typeof useQuery>[0]) => ReturnType<typeof useQuery>, keep the
capturedQueryFn = config.queryFn assignment, and use that precise function type
instead of "as any" so the vi.mocked(useQuery).mockImplementation call is
correctly typed.

In `@app/src/lib/collect-parameter-names.ts`:
- Around line 168-176: In buildParameterSourceMap, normalize each parameter name
before using it as a key: for each name in paramNames trim whitespace into a
nameKey (e.g., const nameKey = name.trim()), skip continue when nameKey is
empty, and use nameKey in the map indexing and deduplication checks (replace
occurrences of name with nameKey in the map[name] lookup and the .some check
against widget.id) so whitespace variants no longer create distinct keys or
empty entries.

In `@app/src/lib/data-transforms.ts`:
- Around line 264-290: The current evaluation in evaluate expression (using
tokens, resolveToken, result, op) evaluates strictly left-to-right, producing
wrong results for mixed precedence; fix by implementing operator precedence:
first scan the tokens and collapse all * and / operations (use resolveToken on
operands and handle divide-by-zero returning null) to produce a reduced token
list, then evaluate the reduced list left-to-right for + and -; ensure you
update any places referencing result/op/tokens and return null for any invalid
operator or failed resolveToken calls.
- Around line 191-199: The count aggregation is using rows.length (overcounting
null/undefined) inside the loop over t.aggregations; change the "count" branch
in the switch (where agg.fn is checked and outKey is computed) to use the
already-computed values.length (or nums.length if you intend to count only
numeric values) instead of rows.length so aggregated[outKey] reflects non-null
column occurrences; update the "count" case in the same block that computes
values and nums to assign aggregated[outKey] = values.length.

In `@app/src/stores/widget-editor-store.ts`:
- Line 376: Guard the persisted transforms before assigning them during
hydration: check Array.isArray(s.transforms) and only use s.transforms when it
is an array, otherwise fall back to an empty array; update the hydration
assignment that currently uses "transforms: (s.transforms as Transform[] |
undefined) ?? []" so the store's transforms property is always an array and
cannot inject non-array values into the Transform editing code.

In `@component/src/components/composed/chart-settings-panel.tsx`:
- Around line 25-27: The PR adds a visible Transform tab (see variable
transformTab and the computed tab array in chart-settings-panel.tsx) but lacks
required visual artifacts; add matching screenshots showing the UI before and
after this change by updating the repository’s .screenshots/before/ and
.screenshots/after/ folders with images that clearly display the settings panel
without the Transform tab (before) and with the Transform tab visible and
selected (after), using the same naming convention and resolution as other
screenshot tests so CI/visual-review can validate the UI change.

In `@component/src/lib/export-utils.ts`:
- Around line 8-16: The CSV formula-injection check currently tests only for
formula chars at position 0 using /^[=@+\-]/; update the detection in the
needsQuoting computation to also catch values with leading whitespace (e.g.
"\t=2+2" or "  +cmd") by testing for leading whitespace before the formula char
(e.g. use /^\s*[=@+\-]/ or test str.trimStart() against /^[=@+\-]/), keeping the
rest of needsQuoting logic intact (refer to the needsQuoting variable and its
regex check in export-utils.ts).

In `@connection/__tests__/connection/query-basic.ts`:
- Around line 11-13: Each test that instantiates Neo4jConnectionModule (created
via getNeo4jAuth()) must ensure the underlying driver is closed to avoid leaking
sockets; update each test (including the ones at lines 37-39, 62-64, 88-90) to
wrap the test body in try/finally and call the connection shutdown in the
finally block—e.g., invoke connection.close() (or connection.driver.close() if
the module exposes the raw driver) to reliably close the Neo4j driver after the
test completes.

---

Outside diff comments:
In `@app/e2e/widget-lab.spec.ts`:
- Around line 98-134: The test currently races waiting for the template to
appear in the UI; instead, after filling saveDialog and before asserting
invisibility, wait for and capture the backend create response (e.g. use
page.waitForResponse or page.waitForRequest to match POST
/api/widget-templates), assert the response status and parse the JSON to extract
the created template id, assign it to templateId, and then continue with UI
assertions (remove the later GET /api/widget-templates and templates.find
logic). Target symbols: the test block named "can save a widget as a template
and see it in Widget Lab", the saveDialog interaction code, the templateName
variable, and the templateId assignment.

In `@app/e2e/widgets.spec.ts`:
- Around line 79-94: The test "should add a JSON viewer widget" races because it
submits before the query preview finishes; after typeInEditor(dialog, page,
"...") use the same run+preview synchronization as other widget tests: click the
dialog's "Run" button (dialog.getByRole("button", { name: "Run" })) and wait for
the preview to be ready (e.g., wait for the preview result/table/preview-region
locator or for any loading spinner to disappear) before asserting the "Add
Widget" button enabled and clicking dialog.getByRole("button", { name: "Add
Widget" }); this ensures the query execution/preview gate is satisfied prior to
submit.

In `@app/src/components/dashboard-container.tsx`:
- Around line 135-140: The selector in scrollToSource does not escape widget
IDs, so IDs with special characters won't match; update the useCallback
scrollToSource (in dashboard-container.tsx) to call CSS.escape on sourceWidgetId
before building the attribute selector (mirroring scroll-to-widget.ts) and then
use
document.querySelector(`[data-widget-id="${escapedId}"]`)?.scrollIntoView(...);
ensure you handle the case where CSS.escape may be undefined in older
environments by still calling it (native browsers support it) and keep the early
return when sourceWidgetId is falsy.

In `@connection/__tests__/connection/query-basic.ts`:
- Around line 20-27: The test's QueryCallback implementation (queryCallback)
currently swallows failures by only logging in onFail; update the onFail handler
used by runQuery to re-throw the received error (or call the test's fail
assertion) instead of just console.error so the test fails when onFail is
invoked; ensure you change both occurrences of the QueryCallback onFail (the one
around lines shown and the second occurrence) so any invocation by runQuery
surfaces as a test failure.

In `@connection/__tests__/connection/query-write.ts`:
- Around line 12-17: The test suite currently only closes the last assigned
Neo4j driver because the shared variable connection gets overwritten; change the
teardown so each driver is closed: either create and close the
Neo4jConnectionModule (or its driver via getDriver().close()) inside each test,
or replace the single afterAll() with an afterEach() that checks the test-scoped
connection (the connection variable or a locally stored array of created
connections) and calls await connection.getDriver().close() for each created
instance; ensure you reference the existing Neo4jConnectionModule/connection and
getDriver().close() calls so no driver instance is left open.

In `@connection/src/postgresql/PostgresAuthenticationModule.ts`:
- Around line 124-127: updateAuthConfig currently only calls
_checkConfigurationConsistency and sets this._authConfig, but does not
re-validate the connection URI format that the constructor enforces, so invalid
URIs can slip in; modify updateAuthConfig to run the same URI/protocol
validation used in the constructor (reuse or call the constructor's URI
validation helper or extract it into a shared method) before assigning
this._authConfig, ensuring the URI/protocol guard (the same checks used when
constructing PostgresAuthenticationModule) is applied and throw/raise the same
error if the URI is invalid.

---

Nitpick comments:
In `@app/e2e/charts.spec.ts`:
- Around line 362-365: The two test.skip markers using test.fixme (notably the
test labeled "PostgreSQL pie chart — fetches data and renders canvas" and the
other test.fixme around lines 995-997) need a short adjacent comment that cites
an issue ID and clear exit criteria; update each test.fixme by adding a comment
directly above it that includes the tracking issue (e.g., ISSUE-1234) and
concise unblock conditions (what must be fixed to re-enable the test, e.g., "CM6
__cmView race fixed and typeInEditor stable in CI"), so reviewers can trace and
close the blind spot and know when to re-enable the test.

In `@app/src/components/widget-editor-modal.tsx`:
- Around line 164-174: Remove the redundant initialization: the useEffect that
calls useWidgetEditorStore.getState().loadFromWidget(widget) / resetForAdd() on
modal open duplicates the later effect that sets all store fields explicitly, so
delete this early effect and rely on the single effect that performs full state
initialization (the effect that sets all state fields and store setters) to
avoid double updates; ensure the remaining effect uses
loadFromWidget/resetForAdd as appropriate or directly sets the store
consistently via useWidgetEditorStore methods (loadFromWidget, resetForAdd) so
there are no conflicting initializations.

In `@app/src/components/widget-editor/transform-editor.tsx`:
- Around line 219-302: The GroupBy UI only reads/edits the first aggregation in
transform.aggregations, so multiple aggregations are ignored; update the UI to
support multiple aggregations (or explicitly document the limitation). Fix by
changing the Group Column/Aggregate/Function blocks (the Selects that read/write
transform.aggregations[0]) to map over transform.aggregations and render a row
per aggregation with controls bound to each aggregation (using index or id) and
call onChange with the updated aggregations array, and include an "Add
aggregation" and "Remove" action to push/splice entries; ensure AGG_FUNCTIONS is
used for each aggregation's function Select and keep transform.type checks
intact.
- Around line 143-148: The code uses an overly verbose conditional type to
extract operator from v; simplify by casting v to the specific variant instead
of the conditional inference — for example, treat v as Transform & { type:
"filter" } (or use Extract<Transform, { type: "filter" }>) and use that cast to
get the operator type for the operator field; update the code around the
operator assignment in transform-editor.tsx (the v variable and Transform type
usage) to use this simpler cast-based extraction.
- Around line 414-424: The list rendering uses the array index as the React key
(in transforms.map -> TransformCard key={i}), which can break reconciliation
when items are inserted/removed; modify the transform data model to include a
stable unique id (e.g., add an id when creating new transforms in whatever
function constructs them) and update rendering to use that stable id
(key={t.id}); ensure any functions that update or remove transforms
(updateTransform, removeTransform) carry or locate transforms by this id rather
than array index so state updates remain correct.

In `@component/src/lib/__tests__/export-utils.test.ts`:
- Around line 9-61: Add regression tests to export-utils.test.ts covering
CSV-injection hardening for values that start with the unsafe prefixes "=", "+",
"-", and "@" so the escapeCsvCell behavior cannot regress; specifically, add
test cases calling escapeCsvCell with inputs beginning with each of those
characters (including variants like a leading quote or whitespace if relevant)
and assert the returned CSV cell is the hardened form produced by escapeCsvCell
(use the same expected output pattern as other tests), referencing the
escapeCsvCell function to locate where to add them.

In `@component/src/lib/cypher-lang/__tests__/cypher-lang-smoke.test.ts`:
- Around line 77-83: Replace the brittle map-size assertion in the test for
tokenTypeToStyleTag with explicit checks for required keys: remove the
Object.keys(tokenTypeToStyleTag).length >= 20 assertion and instead assert that
specific entries exist on tokenTypeToStyleTag (e.g.,
tokenTypeToStyleTag.keyword, tokenTypeToStyleTag.comment,
tokenTypeToStyleTag.stringLiteral and any other essential types such as number,
operator, variable or function used elsewhere). Update the test in
cypher-lang-smoke.test.ts to only assert existence of these named properties
rather than relying on a threshold count.

In `@connection/src/generalized/interfaces.ts`:
- Around line 244-247: Add a required discriminator field to the advanced
options so the union AdvancedConnectionOptions can be type-narrowed: add a
required connectorType: ConnectionTypes.NEO4J to the Neo4jAdvancedOptions
interface and connectorType: ConnectionTypes.POSTGRES to the
PostgresAdvancedOptions interface (or string literals matching the existing
ConnectionTypes enum), then rebuild the AdvancedConnectionOptions union; update
any creation code that constructs these option objects to include the new
connectorType property.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 49f1b96a-cef3-44c0-b4c6-4641d39c837b

📥 Commits

Reviewing files that changed from the base of the PR and between fd7aa8e and 70f9823.

⛔ Files ignored due to path filters (18)

• .claude/.gitignore is excluded by !.claude/**
• .claude/agents/code-reviewer.md is excluded by !.claude/**
• .claude/agents/pr-reviewer.md is excluded by !.claude/**
• .claude/hooks/check-coverage.sh is excluded by !.claude/**
• .claude/hooks/check-credential-logging.sh is excluded by !.claude/**
• .claude/hooks/check-query-safety.sh is excluded by !.claude/**
• .claude/hooks/enforce-e2e.sh is excluded by !.claude/**
• .claude/hooks/format-and-lint.sh is excluded by !.claude/**
• .claude/hooks/session-context.sh is excluded by !.claude/**
• .claude/settings.json is excluded by !.claude/**
• .claude/skills/design-review/skill.md is excluded by !.claude/**
• .claude/skills/harden/SKILL.md is excluded by !.claude/**
• .claude/skills/polish/SKILL.md is excluded by !.claude/**
• .claude/skills/ui-audit/SKILL.md is excluded by !.claude/**
• app/.screenshots/form-widget-403-write-permission.png is excluded by !**/*.png, !app/.screenshots/**
• app/tsconfig.tsbuildinfo is excluded by !app/tsconfig.tsbuildinfo
• docs/content/docs/developer/extending/new-connector.mdx is excluded by !docs/content/**/*.mdx
• package-lock.json is excluded by !**/package-lock.json

📒 Files selected for processing (47)

• .coderabbit.yaml
• .github/workflows/ci.yml
• app/e2e/charts.spec.ts
• app/e2e/widget-lab.spec.ts
• app/e2e/widgets.spec.ts
• app/src/app/(dashboard)/[id]/edit/page.tsx
• app/src/app/(dashboard)/[id]/page.tsx
• app/src/app/globals.css
• app/src/components/card-container.tsx
• app/src/components/dashboard-container.tsx
• app/src/components/widget-editor-modal.tsx
• app/src/components/widget-editor/transform-editor.tsx
• app/src/hooks/__tests__/use-api-keys.test.ts
• app/src/hooks/__tests__/use-connections.test.ts
• app/src/hooks/__tests__/use-dashboards.test.ts
• app/src/hooks/__tests__/use-query-execution.test.ts
• app/src/hooks/__tests__/use-seed-query.test.ts
• app/src/hooks/__tests__/use-users.test.ts
• app/src/hooks/__tests__/use-widget-templates.test.ts
• app/src/hooks/__tests__/use-write-query-execution.test.ts
• app/src/lib/__tests__/collect-parameter-names.test.ts
• app/src/lib/__tests__/data-transforms.test.ts
• app/src/lib/__tests__/scroll-to-widget.test.ts
• app/src/lib/collect-parameter-names.ts
• app/src/lib/data-transforms.ts
• app/src/lib/query-executor.ts
• app/src/lib/scroll-to-widget.ts
• app/src/stores/widget-editor-store.ts
• component/src/components/composed/__tests__/markdown-tables.test.tsx
• component/src/components/composed/chart-settings-panel.tsx
• component/src/components/composed/markdown-widget.tsx
• component/src/lib/__tests__/export-utils.test.ts
• component/src/lib/cypher-lang/__tests__/cypher-lang-smoke.test.ts
• component/src/lib/export-utils.ts
• component/src/utils/index.ts
• connection/__tests__/advanced-connection-options.test.ts
• connection/__tests__/connection/query-basic.ts
• connection/__tests__/connection/query-write.ts
• connection/src/generalized/AuthenticationModule.ts
• connection/src/generalized/interfaces.ts
• connection/src/index.ts
• connection/src/neo4j/Neo4jAuthenticationModule.ts
• connection/src/neo4j/Neo4jConnectionModule.ts
• connection/src/postgresql/PostgresAuthenticationModule.ts
• connection/src/postgresql/PostgresConnectionModule.ts
• eslint.config.js
• package.json

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, let's examine the full file and the context around lines 321-329
cd /tmp && find . -name "page.tsx" -path "*dashboard*edit*" 2>/dev/null | head -5

Repository: alfredo1996/neoboard

Length of output: 46

---

🏁 Script executed:

# Search for the file in the provided context
find . -type f -name "page.tsx" | grep -E "dashboard.*edit" | head -5

Repository: alfredo1996/neoboard

Length of output: 108

---

🏁 Script executed:

# Get a broader view of the file structure
git ls-files | grep -E "edit.*page\.tsx" | head -10

Repository: alfredo1996/neoboard

Length of output: 106

---

🏁 Script executed:

# List files in the app directory to understand structure
git ls-files app/src/app | grep -E "\(dashboard\)" | head -20

Repository: alfredo1996/neoboard

Length of output: 391

---

🏁 Script executed:

# Read the file to understand the implementation
wc -l ./app/src/app/(dashboard)/[id]/edit/page.tsx

Repository: alfredo1996/neoboard

Length of output: 201

---

🏁 Script executed:

# Read the file with line numbers, focusing on the context around lines 321-329
sed -n '300,350p' ./app/src/app/(dashboard)/[id]/edit/page.tsx | cat -n

Repository: alfredo1996/neoboard

Length of output: 222

---

🏁 Script executed:

# Also get a wider view to understand parameterSourceMap and parameter handling
sed -n '1,100p' ./app/src/app/(dashboard)/[id]/edit/page.tsx | cat -n

Repository: alfredo1996/neoboard

Length of output: 220

---

🏁 Script executed:

# Read the file with proper quoting
wc -l 'app/src/app/(dashboard)/[id]/edit/page.tsx'

Repository: alfredo1996/neoboard

Length of output: 110

---

🏁 Script executed:

# Read the context around lines 321-329
sed -n '300,350p' 'app/src/app/(dashboard)/[id]/edit/page.tsx' | cat -n

Repository: alfredo1996/neoboard

Length of output: 2047

---

🏁 Script executed:

# Read the beginning to understand parameterSourceMap and setup
sed -n '1,150p' 'app/src/app/(dashboard)/[id]/edit/page.tsx' | cat -n

Repository: alfredo1996/neoboard

Length of output: 6422

---

🏁 Script executed:

# Search for where widget queries are executed and how cache keys are built
rg -A 5 -B 5 'widget-query' 'app/src' --type ts --type tsx | head -100

Repository: alfredo1996/neoboard

Length of output: 91

---

🏁 Script executed:

# Look for the useWidgetQuery hook or similar
rg -A 10 'useWidgetQuery\|queryKey.*widget' 'app/src' --type ts --type tsx | head -80

Repository: alfredo1996/neoboard

Length of output: 91

---

🏁 Script executed:

# Search for parameterSourceMap usage
rg -A 3 -B 3 'parameterSourceMap' 'app/src/app/(dashboard)/[id]/edit/page.tsx'

Repository: alfredo1996/neoboard

Length of output: 470

---

🏁 Script executed:

# Search for widget-query usage more broadly, without typescript file type restriction
rg -B 5 -A 10 'widget-query' 'app/src' | head -150

Repository: alfredo1996/neoboard

Length of output: 11336

---

🏁 Script executed:

# Look for the hook that executes widget queries
rg -B 3 -A 15 'useWidgetQuery\|function.*widget.*query' 'app/src' | head -120

Repository: alfredo1996/neoboard

Length of output: 46

---

🏁 Script executed:

# Search for where queryKey is built with widget parameters
rg -B 5 -A 8 'queryKey.*widget\|useQuery.*widget' 'app/src' | head -150

Repository: alfredo1996/neoboard

Length of output: 46

---

🏁 Script executed:

# Check how widget.params is used and if it's always present
rg -B 3 -A 3 'widget\.params' 'app/src/app/(dashboard)/[id]/edit/page.tsx'

Repository: alfredo1996/neoboard

Length of output: 46

---

🏁 Script executed:

# Look at the DashboardWidget type to see what fields are available
rg -B 2 -A 15 'type DashboardWidget\|interface DashboardWidget' 'app/src/lib'

Repository: alfredo1996/neoboard

Length of output: 46

---

🏁 Script executed:

# Check the widget-editor-modal to see how params are handled when editing
rg -B 5 -A 10 'cachedPreviewData' 'app/src/components/widget-editor-modal.tsx'

Repository: alfredo1996/neoboard

Length of output: 46

---

🏁 Script executed:

# Find the schema file
find 'app/src' -name 'schema*' -type f

Repository: alfredo1996/neoboard

Length of output: 331

---

🏁 Script executed:

# Look at the use-widget-query hook more carefully
cat 'app/src/hooks/use-widget-query.ts' | head -80

Repository: alfredo1996/neoboard

Length of output: 2334

---

🏁 Script executed:

# Check the dashboard-container for comparison on how it handles the partial key lookup
sed -n '1,50p' 'app/src/components/dashboard-container.tsx' | cat -n

Repository: alfredo1996/neoboard

Length of output: 1749

---

🏁 Script executed:

# Read more of the use-widget-query to see the full queryKey construction
sed -n '70,130p' 'app/src/hooks/use-widget-query.ts'

Repository: alfredo1996/neoboard

Length of output: 1848

---

🏁 Script executed:

# Check if widget.params is ever set or always derived from parameter store
sed -n '200,350p' 'app/src/app/(dashboard)/[id]/edit/page.tsx' | cat -n

Repository: alfredo1996/neoboard

Length of output: 6264

---

🏁 Script executed:

# Look at the complete exportWidgetCsv function to see the full context
sed -n '90,130p' 'app/src/components/dashboard-container.tsx' | cat -n

Repository: alfredo1996/neoboard

Length of output: 1882

---

🏁 Script executed:

# Verify how TanStack React Query orders getQueriesData results
web_search "TanStack React Query getQueriesData order stable results"

Repository: alfredo1996/neoboard

Length of output: 114

---

🏁 Script executed:

# Check if there's any test or documentation about cache entry ordering
rg -B 2 -A 5 'getQueriesData' 'app/src' --type ts | grep -A 5 -B 5 'test\|spec\|order'

Repository: alfredo1996/neoboard

Length of output: 46

---

🏁 Script executed:

# Confirm the widget.params field exists and is used
sed -n '1,50p' 'app/src/lib/db/schema.ts' | cat -n

Repository: alfredo1996/neoboard

Length of output: 1882

---

🏁 Script executed:

# Search for DashboardWidget type definition
rg -A 20 'export.*type DashboardWidget' 'app/src/lib/db/schema.ts'

Repository: alfredo1996/neoboard

Length of output: 46

---

🏁 Script executed:

# Check if widget.params is ever populated or used elsewhere
rg 'widget\.params\s*=' 'app/src' | head -20

Repository: alfredo1996/neoboard

Length of output: 46

---

🏁 Script executed:

# Look at the card-container to see if it also has this issue
sed -n '60,120p' 'app/src/components/card-container.tsx' | cat -n

Repository: alfredo1996/neoboard

Length of output: 2669

---

🏁 Script executed:

# Find and read the DashboardWidget type definition
rg -B 5 -A 25 'type DashboardWidget\|interface DashboardWidget' 'app/src/lib/db/schema.ts'

Repository: alfredo1996/neoboard

Length of output: 46

---

🏁 Script executed:

# Check how widget queries are actually constructed - look at the actual query execution
sed -n '100,150p' 'app/src/hooks/use-widget-query.ts'

Repository: alfredo1996/neoboard

Length of output: 2050

---

🏁 Script executed:

# Verify the actual cache key includes params as the 4th element
sed -n '110,145p' 'app/src/hooks/use-widget-query.ts' | cat -n

Repository: alfredo1996/neoboard

Length of output: 1809

---

🏁 Script executed:

# Continue reading use-widget-query to see the full queryKey construction with params
sed -n '145,185p' 'app/src/hooks/use-widget-query.ts' | cat -n

Repository: alfredo1996/neoboard

Length of output: 1812

---

🏁 Script executed:

# Search for where DashboardWidget type is exported/defined
find 'app/src/lib' -name '*.ts' -exec grep -l 'DashboardWidget' {} \;

Repository: alfredo1996/neoboard

Length of output: 247

---

🏁 Script executed:

# Check what fields are in DashboardWidget
rg 'export.*const.*=.*pgTable.*"dashboard_widget' 'app/src/lib/db/schema.ts' -A 40 | head -60

Repository: alfredo1996/neoboard

Length of output: 46

---

🏁 Script executed:

# Find the dashboard_widget table definition
rg -A 50 'export const dashboardWidgets = pgTable' 'app/src/lib/db/schema.ts'

Repository: alfredo1996/neoboard

Length of output: 46

---

🏁 Script executed:

# Look for the Drizzle schema type inference for DashboardWidget
rg -B 3 -A 15 'type DashboardWidget' 'app/src/lib/db/schema.ts'

Repository: alfredo1996/neoboard

Length of output: 46

---

🏁 Script executed:

# Search the entire schema file for dashboard widget related definitions
cat 'app/src/lib/db/schema.ts' | grep -A 30 'dashboard' | head -100

Repository: alfredo1996/neoboard

Length of output: 2943

---

🏁 Script executed:

# Try searching for DashboardWidget without type keyword
rg 'DashboardWidget' 'app/src/lib/db/schema.ts' -B 5 -A 5

Repository: alfredo1996/neoboard

Length of output: 838

---

🏁 Script executed:

# Check how widget.params is set when editing or adding widgets
rg -B 5 -A 5 'params.*:' 'app/src/app/(dashboard)/[id]/edit/page.tsx' | head -40

Repository: alfredo1996/neoboard

Length of output: 384

---

🏁 Script executed:

# Look at widget-editor-modal to see if it sets params
rg -B 3 -A 10 'params.*=' 'app/src/components/widget-editor-modal.tsx' | head -50

Repository: alfredo1996/neoboard

Length of output: 820

---

🏁 Script executed:

# Verify React Query's getQueriesData documentation behavior in actual usage
rg -B 10 -A 5 'getQueriesData' 'app/src/components/dashboard-container.tsx'

Repository: alfredo1996/neoboard

Length of output: 803

---

🏁 Script executed:

# Verify if widget.params is populated in the editing widget before openEditWidget is called
sed -n '130,160p' 'app/src/app/(dashboard)/[id]/edit/page.tsx' | cat -n

Repository: alfredo1996/neoboard

Length of output: 1587

---

🏁 Script executed:

# Check the WidgetEditorModal to see what cached data it receives and how it's used
rg -B 5 -A 10 'cachedPreviewData\|initialPreviewData' 'app/src/components/widget-editor-modal.tsx' | head -60

Repository: alfredo1996/neoboard

Length of output: 46

---

Resolve cache key mismatch for consistent preview data.

getQueriesData with partial key ["widget-query", connectionId, query] matches all cache entries with varying params. Taking [0] returns nondeterministic results — if the same widget/query has cached results for different parameter states, the modal opens with stale data from whichever entry happens to be first.

Include the current parameter state in the cache key lookup, or at minimum select the most recent matching entry instead of the first.

This affects both openEditWidget (edit/page.tsx lines 122–134) and exportWidgetCsv (dashboard-container.tsx).

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/src/app/`(dashboard)/[id]/edit/page.tsx around lines 321 - 329, The
preview uses queryClient.getQueriesData with a partial key ["widget-query",
widget.connectionId, widget.query], which can return nondeterministic entries
when the same widget/query has cached results for different parameter states;
update openEditWidget to include the current parameter state in the cache lookup
(e.g., add widget.params or the current parameter values to the queryKey) so you
retrieve the exact cached entry for the active parameters, or if you
intentionally need to match multiple entries, pick the most recent matching
entry (by comparing timestamps/metadata on the returned entries) instead of
taking entries[0]; apply the same fix in exportWidgetCsv to ensure it looks up
the cache with the full parameter-aware key (or chooses the most recent matching
entry) so both preview and CSV export use consistent, parameter-specific cached
data.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Skip transforms for graph widgets in the live-data path too.

Line 339 already guards graph widgets because their mapped payload is not tabular, but this branch still sends live graph results through applyTransforms. A graph widget with saved transforms will hit the wrong pipeline after query execution and can fail at render time.

Suggested fix

-  const transformedData = dataTransforms.length
+  const transformedData =
+    dataTransforms.length && widget.chartType !== "graph"
     ? applyTransforms(
         mappedData as Record<string, unknown>[],
         dataTransforms,
         allParamValues,
       )
     : mappedData;

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	const mappedData = chartConfig.transformWithMapping(rawData, columnMapping);
	const transformedData = dataTransforms.length
	? applyTransforms(
	mappedData as Record<string, unknown>[],
	dataTransforms,
	allParamValues,
	)
	: mappedData;
	const mappedData = chartConfig.transformWithMapping(rawData, columnMapping);
	const transformedData =
	dataTransforms.length && widget.chartType !== "graph"
	? applyTransforms(
	mappedData as Record<string, unknown>[],
	dataTransforms,
	allParamValues,
	)
	: mappedData;

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/src/components/card-container.tsx` around lines 558 - 565, The live-data
branch is still sending graph widget payloads through applyTransforms, causing
non-tabular mappedData to be transformed incorrectly; update the transformedData
assignment so transforms are applied only when dataTransforms exist AND the
mappedData is tabular (e.g., Array.isArray(mappedData) and items are plain
objects) or when the chartConfig explicitly indicates a non-graph widget (use an
existing flag like chartConfig.isGraph if present); change the conditional that
builds transformedData (the code that calls chartConfig.transformWithMapping,
mappedData, applyTransforms, dataTransforms, and allParamValues) to skip
applyTransforms for graph/non-tabular mappedData and return mappedData directly
in that case.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Rename mapping parsing has edge cases.

Column names containing = or , will be incorrectly parsed. This is a minor UX issue for unusual column names.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/src/components/widget-editor/transform-editor.tsx` around lines 332 -
353, The current parsing in the Input onChange for transform.type ===
"renameColumns" (reading transform.mapping) naively splits on commas and '='
which breaks for column names containing those characters; update the handler
for the Input change (the onChange that builds mapping from e.target.value) to
use a robust parse strategy — either require and parse a JSON object string or
implement a small CSV/key-value parser that supports quoted values and escaping
(respecting commas and equals inside quotes) — then set onChange({ ...transform,
mapping }) with the reliably parsed Record<string,string>; keep references to
transform.mapping, the Input onChange, and the mapping construction so reviewers
can find the change.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify no unannotated `any` usage remains in this test file
fd -a "use-seed-query.test.ts" | xargs -I{} rg -nP '\bas\s+any\b|\b:\s*any\b' "{}"

Repository: alfredo1996/neoboard

Length of output: 85

---

Remove untyped any cast on line 201.

The as any cast bypasses strict typing and violates the TypeScript guideline requiring comments for any any usage. Replace with proper typing to maintain type safety.

♻️ Proposed fix

-      vi.mocked(useQuery).mockImplementation(((
+      vi.mocked(useQuery).mockImplementation((
         config: Record<string, unknown>,
       ) => {
         capturedQueryFn = config.queryFn as typeof capturedQueryFn;
-        return { data: null, isLoading: false };
-      }) as any);
+        return { data: null, isLoading: false } as ReturnType<typeof useQuery>;
+      }) as unknown as typeof useQuery);

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	vi.mocked(useQuery).mockImplementation(((
	config: Record<string, unknown>,
	) => {
	capturedQueryFn = config.queryFn as typeof capturedQueryFn;
	return { data: null, isLoading: false };
	}) as any);
	vi.mocked(useQuery).mockImplementation((
	config: Record<string, unknown>,
	) => {
	capturedQueryFn = config.queryFn as typeof capturedQueryFn;
	return { data: null, isLoading: false } as ReturnType<typeof useQuery>;
	}) as unknown as typeof useQuery);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/src/hooks/__tests__/use-seed-query.test.ts` around lines 196 - 201,
Replace the untyped "as any" cast by giving the mock function a proper
TypeScript signature: declare the mock implementation parameter using
Parameters<typeof useQuery>[0] and its return type using ReturnType<typeof
useQuery>, e.g. implement the mock as (config: Parameters<typeof useQuery>[0])
=> ReturnType<typeof useQuery>, keep the capturedQueryFn = config.queryFn
assignment, and use that precise function type instead of "as any" so the
vi.mocked(useQuery).mockImplementation call is correctly typed.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Normalize parameter names before indexing the source map.

buildParameterSourceMap currently indexes raw names; whitespace variants become distinct keys and can break parameter-source lookups. Trim and skip empty names before map insertion.

Suggested fix

-      for (const name of paramNames) {
-        if (!map[name]) {
-          map[name] = [];
+      for (const name of paramNames) {
+        const normalizedName = name.trim();
+        if (!normalizedName) continue;
+        if (!map[normalizedName]) {
+          map[normalizedName] = [];
         }
         // Deduplicate: same widget can appear in paramNames multiple times
         // (e.g. top-level parameterMapping + a rule referencing the same param)
-        if (!map[name].some((s) => s.widgetId === widget.id)) {
-          map[name].push(source);
+        if (!map[normalizedName].some((s) => s.widgetId === widget.id)) {
+          map[normalizedName].push(source);
         }
       }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/src/lib/collect-parameter-names.ts` around lines 168 - 176, In
buildParameterSourceMap, normalize each parameter name before using it as a key:
for each name in paramNames trim whitespace into a nameKey (e.g., const nameKey
= name.trim()), skip continue when nameKey is empty, and use nameKey in the map
indexing and deduplication checks (replace occurrences of name with nameKey in
the map[name] lookup and the .some check against widget.id) so whitespace
variants no longer create distinct keys or empty entries.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Expression evaluation needs operator precedence.

These formulas are reduced strictly left-to-right, so 2 + 3 * 4 becomes 20 instead of 14. Any calculated column that mixes +/- with *// will return the wrong value unless you either implement precedence or reject mixed-precedence expressions up front.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/src/lib/data-transforms.ts` around lines 264 - 290, The current
evaluation in evaluate expression (using tokens, resolveToken, result, op)
evaluates strictly left-to-right, producing wrong results for mixed precedence;
fix by implementing operator precedence: first scan the tokens and collapse all
* and / operations (use resolveToken on operands and handle divide-by-zero
returning null) to produce a reduced token list, then evaluate the reduced list
left-to-right for + and -; ensure you update any places referencing
result/op/tokens and return null for any invalid operator or failed resolveToken
calls.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Guard transforms before hydrating the store.

widget.settings is persisted JSON. If s.transforms is anything other than an array, this injects an invalid value into Zustand and the transform editor will fail on the first array operation. Use an Array.isArray guard here.

Suggested fix

-      transforms: (s.transforms as Transform[] | undefined) ?? [],
+      transforms: Array.isArray(s.transforms)
+        ? (s.transforms as Transform[])
+        : [],

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	transforms: (s.transforms as Transform[] | undefined) ?? [],
	transforms: Array.isArray(s.transforms)
	? (s.transforms as Transform[])
	: [],

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/src/stores/widget-editor-store.ts` at line 376, Guard the persisted
transforms before assigning them during hydration: check
Array.isArray(s.transforms) and only use s.transforms when it is an array,
otherwise fall back to an empty array; update the hydration assignment that
currently uses "transforms: (s.transforms as Transform[] | undefined) ?? []" so
the store's transforms property is always an array and cannot inject non-array
values into the Transform editing code.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Add required before/after screenshots for the new Transform tab.

This is a visible UI change, but the PR context here doesn’t include matching screenshot artifacts. Please attach/update screenshots in the required folders before merge.

As per coding guidelines, "Screenshot before AND after any visual change (.screenshots/before/, .screenshots/after/)."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@component/src/components/composed/chart-settings-panel.tsx` around lines 25 -
27, The PR adds a visible Transform tab (see variable transformTab and the
computed tab array in chart-settings-panel.tsx) but lacks required visual
artifacts; add matching screenshots showing the UI before and after this change
by updating the repository’s .screenshots/before/ and .screenshots/after/
folders with images that clearly display the settings panel without the
Transform tab (before) and with the Transform tab visible and selected (after),
using the same naming convention and resolution as other screenshot tests so
CI/visual-review can validate the UI change.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Harden CSV formula-injection detection for whitespace-prefixed payloads.

Line [13] only matches formulas at char 0. Values like "\t=2+2" or " +cmd" can bypass this check.

Suggested patch

 export function escapeCsvCell(value: unknown): string {
   if (value === null || value === undefined) return "";
   const str = typeof value === "object" ? JSON.stringify(value) : String(value);
+  const isFormulaLike = /^[\t ]*[=@+\-]/.test(str);
+  const safe = isFormulaLike ? `'${str}` : str;
   const needsQuoting =
-    str.includes(",") ||
-    str.includes('"') ||
-    str.includes("\n") ||
-    str.includes("\r") ||
-    /^[=@+\-]/.test(str);
+    safe.includes(",") ||
+    safe.includes('"') ||
+    safe.includes("\n") ||
+    safe.includes("\r");
   if (needsQuoting) {
-    return `"${str.replace(/"/g, '""')}"`;
+    return `"${safe.replace(/"/g, '""')}"`;
   }
-  return str;
+  return safe;
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@component/src/lib/export-utils.ts` around lines 8 - 16, The CSV
formula-injection check currently tests only for formula chars at position 0
using /^[=@+\-]/; update the detection in the needsQuoting computation to also
catch values with leading whitespace (e.g. "\t=2+2" or "  +cmd") by testing for
leading whitespace before the formula char (e.g. use /^\s*[=@+\-]/ or test
str.trimStart() against /^[=@+\-]/), keeping the rest of needsQuoting logic
intact (refer to the needsQuoting variable and its regex check in
export-utils.ts).

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Close each Neo4j driver opened by these tests.

Every test constructs a Neo4jConnectionModule, but none of them close the underlying driver. That leaks sockets across the suite and can keep Jest workers alive. Add try/finally per test or an afterEach that closes the current connection.

Also applies to: 37-39, 62-64, 88-90

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@connection/__tests__/connection/query-basic.ts` around lines 11 - 13, Each
test that instantiates Neo4jConnectionModule (created via getNeo4jAuth()) must
ensure the underlying driver is closed to avoid leaking sockets; update each
test (including the ones at lines 37-39, 62-64, 88-90) to wrap the test body in
try/finally and call the connection shutdown in the finally block—e.g., invoke
connection.close() (or connection.driver.close() if the module exposes the raw
driver) to reliably close the Neo4j driver after the test completes.

[coderabbitai]

🧹 Nitpick comments (1)

app/vitest.setup.tsx (1)

55-67: next/dynamic mock has a race condition that may cause flaky tests.

The mock calls loader().then() asynchronously, but the returned component immediately checks Component synchronously. On first render, Component is always null because the promise hasn't resolved yet.

This means tests must use await findBy* or waitFor queries to wait for the dynamic component to appear. If tests use getBy* queries immediately after render, they'll find the fallback <div data-testid="dynamic-loading" /> instead.

If synchronous rendering is intended for simpler tests:

♻️ Proposed fix for synchronous dynamic loading

 vi.mock("next/dynamic", () => ({
   __esModule: true,
-  default: (loader: () => Promise<{ default: React.ComponentType }>, _opts?: unknown) => {
-    // In test environment, resolve the dynamic import synchronously
-    let Component: React.ComponentType | null = null;
-    loader().then((mod) => {
-      Component = mod.default;
-    });
-    return (props: Record<string, unknown>) =>
-      Component ? <Component {...props} /> : <div data-testid="dynamic-loading" />;
+  default: (loader: () => Promise<{ default: React.ComponentType }>, opts?: { loading?: () => React.ReactElement }) => {
+    // Return a component that lazily resolves the import
+    const LazyComponent = React.lazy(loader);
+    return (props: Record<string, unknown>) => (
+      <React.Suspense fallback={opts?.loading?.() ?? <div data-testid="dynamic-loading" />}>
+        <LazyComponent {...props} />
+      </React.Suspense>
+    );
   },
 }));

Alternatively, document that tests using dynamic components must await their appearance.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/vitest.setup.tsx` around lines 55 - 67, The current
vi.mock("next/dynamic") has a race: it calls loader().then(...) asynchronously
so the returned render function always sees Component as null on first render;
change the mock so the factory immediately calls loader() and if the call
returns a synchronous module (i.e., an object with a default export), set
Component immediately and return the component; if loader() returns a Promise,
throw a clear error instructing the test to either provide a synchronous loader
or use async queries (findBy/waitFor) — update the mock around symbols vi.mock,
loader, Component, and the fallback data-testid="dynamic-loading" to implement
this behavior.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@app/vitest.setup.tsx`:
- Around line 55-67: The current vi.mock("next/dynamic") has a race: it calls
loader().then(...) asynchronously so the returned render function always sees
Component as null on first render; change the mock so the factory immediately
calls loader() and if the call returns a synchronous module (i.e., an object
with a default export), set Component immediately and return the component; if
loader() returns a Promise, throw a clear error instructing the test to either
provide a synchronous loader or use async queries (findBy/waitFor) — update the
mock around symbols vi.mock, loader, Component, and the fallback
data-testid="dynamic-loading" to implement this behavior.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: a0f7df6f-5c5d-40c6-9775-8342871e4ba2

📥 Commits

Reviewing files that changed from the base of the PR and between 70f9823 and 86440c1.

⛔ Files ignored due to path filters (1)

• app/package-lock.json is excluded by !**/package-lock.json

📒 Files selected for processing (5)

• CLAUDE.md
• app/package.json
• app/src/components/widget-editor/__tests__/transform-editor.test.tsx
• app/vitest.config.ts
• app/vitest.setup.tsx

✅ Files skipped from review due to trivial changes (1)

• app/package.json

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (4)

app/src/lib/__tests__/widget-utils.test.ts (1)

63-70: Add one explicit case for updatedAt missing on an existing template entry.

You already test “template missing”; add “template present but updatedAt absent/null” to lock Line 22 behavior.

Suggested test addition

 describe("isWidgetTemplateOutdated", () => {
@@
   it("returns false when template is not in the map", () => {
@@
     expect(isWidgetTemplateOutdated(widget, undefined)).toBe(false);
   });
+
+  it("returns false when template exists but updatedAt is missing", () => {
+    const widget = makeWidget({
+      templateId: "t1",
+      templateSyncedAt: "2025-01-01T00:00:00Z",
+    });
+    expect(isWidgetTemplateOutdated(widget, { t1: {} })).toBe(false);
+    expect(
+      isWidgetTemplateOutdated(widget, { t1: { updatedAt: null } }),
+    ).toBe(false);
+  });

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/src/lib/__tests__/widget-utils.test.ts` around lines 63 - 70, Add a test
that covers the case where the template entry exists in the templates map but
its updatedAt is missing/null: call isWidgetTemplateOutdated with a widget
produced by makeWidget (e.g., templateId "t1", templateSyncedAt set) and a
templates map containing "t1": { /* no updatedAt or updatedAt: null */ } and
assert the result matches the expected behavior (false per Line 22). This
ensures isWidgetTemplateOutdated handles an existing template entry with missing
updatedAt.

app/src/lib/__tests__/card-utils.test.ts (1)

60-78: Add regression test for disabled-config precedence.

Please add a case where stylingConfig.enabled === false and colorThresholds is non-empty, asserting undefined. This locks in the intended precedence and prevents legacy fallback from re-enabling styling.

Suggested test case

 describe("resolveStylingConfig", () => {
@@
   it("returns undefined when stylingConfig is disabled and no legacy thresholds", () => {
     const config: StylingConfig = { enabled: false, rules: [] };
     expect(resolveStylingConfig(config, undefined)).toBeUndefined();
   });
+
+  it("keeps styling disabled even if legacy thresholds exist", () => {
+    const config: StylingConfig = { enabled: false, rules: [] };
+    const legacy = JSON.stringify([{ value: 100, color: "red" }]);
+    expect(resolveStylingConfig(config, legacy)).toBeUndefined();
+  });

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/src/lib/__tests__/card-utils.test.ts` around lines 60 - 78, Add a
regression test to assert disabled-config precedence: create a StylingConfig
object with enabled: false and non-empty colorThresholds (legacy JSON string)
and call resolveStylingConfig(stylingConfig, colorThresholds); assert the result
is undefined. Reference resolveStylingConfig and the StylingConfig shape to
ensure the test covers the case where stylingConfig.enabled === false even when
a legacy colorThresholds string is provided.

app/src/lib/card-utils.ts (1)

13-15: Tighten record detection in column extraction.

Line 13–Line 15 accepts arrays as objects and can return numeric keys ("0", "1"). Add an array guard to keep this function record-only.

Proposed refactor

   const first = records[0] as Record<string, unknown> | undefined;
-  if (!first || typeof first !== "object") return [];
+  if (!first || typeof first !== "object" || Array.isArray(first)) return [];
   return Object.keys(first);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/src/lib/card-utils.ts` around lines 13 - 15, The code currently treats
arrays as objects and can return numeric keys; update the guard around the
extracted first record (the variable first from records) to exclude arrays and
nulls before returning Object.keys(first). Concretely, in the column-extraction
logic replace the condition with one that checks first is truthy, typeof first
=== "object", first !== null, and !Array.isArray(first) (e.g., if (!first ||
typeof first !== "object" || Array.isArray(first)) return [];), then safely
return Object.keys(first).

app/src/lib/__tests__/widget-actions.test.ts (1)

141-158: Add a regression test for rules-mode navigation without top-level targetPageId.

Line 141 currently validates rules only for set-parameter. Add a case where actionRules are provided for navigation while legacy top-level fields are empty, so rules-mode behavior stays protected.

Suggested test addition

 describe("buildClickActionConfig", () => {
+  it("uses actionRules without requiring legacy top-level fields", () => {
+    const rules: ClickActionRule[] = [
+      {
+        id: "r-nav",
+        type: "set-parameter-and-navigate",
+        parameterMapping: { parameterName: "p1", sourceField: "col1" },
+        targetPageId: "page-1",
+      },
+    ];
+
+    const result = buildClickActionConfig({
+      ...base,
+      clickActionType: "set-parameter-and-navigate",
+      parameterName: "",
+      sourceField: "",
+      targetPageId: "",
+      layout: {
+        version: 2,
+        pages: [{ id: "page-1", title: "Page 1", widgets: [], gridLayout: [] }],
+      },
+      actionRules: rules,
+    });
+
+    expect(result).toBeDefined();
+    expect(result!.rules).toEqual(rules);
+  });

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/src/lib/__tests__/widget-actions.test.ts` around lines 141 - 158, Add a
regression test that ensures rules-mode navigation works when there is no
top-level targetPageId: create a ClickActionRule with type "navigate" (e.g., {
id: "r-nav", type: "navigate", targetPageId: "pageX", parameterMapping: {...} })
and call buildClickActionConfig with the test "base" object but without
top-level targetPageId/legacy navigation fields and with actionRules set to that
rule; assert the returned config has result!.rules equal to the rules,
result!.type is "navigate", and legacy top-level fields like
result!.targetPageId are undefined to confirm rules-mode is honored.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@app/src/lib/card-utils.ts`:
- Around line 25-27: The code currently returns
migrateColorThresholds(colorThresholds) even when a stylingConfig object exists
with enabled: false; change the early-return to honor any explicitly provided
stylingConfig by checking for its presence (e.g., if (stylingConfig != null)
return stylingConfig) so that migration only runs when stylingConfig is absent,
then keep the legacy branch (if (typeof colorThresholds === "string" &&
colorThresholds.trim()) return migrateColorThresholds(colorThresholds)); update
references to stylingConfig, colorThresholds, and migrateColorThresholds
accordingly.

In `@app/src/lib/widget-actions.ts`:
- Around line 43-66: The current validation block in widget-actions.ts
(variables needsParam, needsPage, trimmedParamName, trimmedSourceField,
trimmedTargetPageId and checks referencing clickActionType and layout)
incorrectly rejects rule-based configs when actionRules is present; change the
logic in the function that computes the resolved action so that if actionRules
(or a non-empty actionRules array) exists you skip the legacy top-level field
guards (the needsParam/needsPage checks and the layout page-id validation) and
allow rule-only configurations to pass, while preserving the existing validation
path when actionRules is absent.

---

Nitpick comments:
In `@app/src/lib/__tests__/card-utils.test.ts`:
- Around line 60-78: Add a regression test to assert disabled-config precedence:
create a StylingConfig object with enabled: false and non-empty colorThresholds
(legacy JSON string) and call resolveStylingConfig(stylingConfig,
colorThresholds); assert the result is undefined. Reference resolveStylingConfig
and the StylingConfig shape to ensure the test covers the case where
stylingConfig.enabled === false even when a legacy colorThresholds string is
provided.

In `@app/src/lib/__tests__/widget-actions.test.ts`:
- Around line 141-158: Add a regression test that ensures rules-mode navigation
works when there is no top-level targetPageId: create a ClickActionRule with
type "navigate" (e.g., { id: "r-nav", type: "navigate", targetPageId: "pageX",
parameterMapping: {...} }) and call buildClickActionConfig with the test "base"
object but without top-level targetPageId/legacy navigation fields and with
actionRules set to that rule; assert the returned config has result!.rules equal
to the rules, result!.type is "navigate", and legacy top-level fields like
result!.targetPageId are undefined to confirm rules-mode is honored.

In `@app/src/lib/__tests__/widget-utils.test.ts`:
- Around line 63-70: Add a test that covers the case where the template entry
exists in the templates map but its updatedAt is missing/null: call
isWidgetTemplateOutdated with a widget produced by makeWidget (e.g., templateId
"t1", templateSyncedAt set) and a templates map containing "t1": { /* no
updatedAt or updatedAt: null */ } and assert the result matches the expected
behavior (false per Line 22). This ensures isWidgetTemplateOutdated handles an
existing template entry with missing updatedAt.

In `@app/src/lib/card-utils.ts`:
- Around line 13-15: The code currently treats arrays as objects and can return
numeric keys; update the guard around the extracted first record (the variable
first from records) to exclude arrays and nulls before returning
Object.keys(first). Concretely, in the column-extraction logic replace the
condition with one that checks first is truthy, typeof first === "object", first
!== null, and !Array.isArray(first) (e.g., if (!first || typeof first !==
"object" || Array.isArray(first)) return [];), then safely return
Object.keys(first).

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: fedf3c64-8e3a-423a-bb9e-e18dc4d2987a

📥 Commits

Reviewing files that changed from the base of the PR and between 86440c1 and e67dc47.

📒 Files selected for processing (6)

• app/src/lib/__tests__/card-utils.test.ts
• app/src/lib/__tests__/widget-actions.test.ts
• app/src/lib/__tests__/widget-utils.test.ts
• app/src/lib/card-utils.ts
• app/src/lib/widget-actions.ts
• app/src/lib/widget-utils.ts

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Honor explicit disabled styling before legacy fallback.

On Line 25–Line 27, a defined stylingConfig with enabled: false can still be replaced by migrated legacy thresholds, which may re-enable styling unexpectedly. Legacy migration should run only when stylingConfig is absent.

Proposed fix

 export function resolveStylingConfig(
   stylingConfig?: StylingConfig,
   colorThresholds?: string,
 ): StylingConfig | undefined {
-  if (stylingConfig?.enabled) return stylingConfig;
+  if (stylingConfig) {
+    return stylingConfig.enabled ? stylingConfig : undefined;
+  }
   if (typeof colorThresholds === "string" && colorThresholds.trim()) {
     return migrateColorThresholds(colorThresholds);
   }
   return undefined;
 }

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	if (stylingConfig?.enabled) return stylingConfig;
	if (typeof colorThresholds === "string" && colorThresholds.trim()) {
	return migrateColorThresholds(colorThresholds);
	export function resolveStylingConfig(
	stylingConfig?: StylingConfig,
	colorThresholds?: string,
	): StylingConfig | undefined {
	if (stylingConfig) {
	return stylingConfig.enabled ? stylingConfig : undefined;
	}
	if (typeof colorThresholds === "string" && colorThresholds.trim()) {
	return migrateColorThresholds(colorThresholds);
	}
	return undefined;
	}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/src/lib/card-utils.ts` around lines 25 - 27, The code currently returns
migrateColorThresholds(colorThresholds) even when a stylingConfig object exists
with enabled: false; change the early-return to honor any explicitly provided
stylingConfig by checking for its presence (e.g., if (stylingConfig != null)
return stylingConfig) so that migration only runs when stylingConfig is absent,
then keep the legacy branch (if (typeof colorThresholds === "string" &&
colorThresholds.trim()) return migrateColorThresholds(colorThresholds)); update
references to stylingConfig, colorThresholds, and migrateColorThresholds
accordingly.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Rules-mode is incorrectly gated by legacy field validation.

When actionRules are present, Line 43–66 still requires top-level fields based on clickActionType. This can return undefined for valid rule-based configs (notably navigate variants with empty legacy targetPageId).

Proposed fix

 export function buildClickActionConfig(opts: {
@@
   const {
@@
     actionRules = [],
   } = opts;
+  const hasRules = actionRules.length > 0;

   if (!clickActionEnabled || !chartSupportsClickAction(chartType))
     return undefined;
@@
   const trimmedParamName = parameterName.trim();
   const trimmedSourceField = sourceField.trim();
   const trimmedTargetPageId = targetPageId.trim();
-
-  if (needsParam && !trimmedParamName) return undefined;
-
   const resolvedSourceField = chartType === "table" ? "" : trimmedSourceField;
-
-  if (needsParam && chartType !== "table" && !resolvedSourceField)
-    return undefined;
-
-  if (needsPage && !trimmedTargetPageId) return undefined;
-
-  if (needsPage && layout) {
-    const validPageIds = new Set((layout.pages ?? []).map((p) => p.id));
-    if (!validPageIds.has(trimmedTargetPageId)) return undefined;
+  if (!hasRules) {
+    if (needsParam && !trimmedParamName) return undefined;
+    if (needsParam && chartType !== "table" && !resolvedSourceField)
+      return undefined;
+    if (needsPage && !trimmedTargetPageId) return undefined;
+    if (needsPage && layout) {
+      const validPageIds = new Set((layout.pages ?? []).map((p) => p.id));
+      if (!validPageIds.has(trimmedTargetPageId)) return undefined;
+    }
   }

   return {
-    type: actionRules.length > 0 ? actionRules[0].type : clickActionType,
-    ...(needsParam && actionRules.length === 0
+    type: hasRules ? actionRules[0].type : clickActionType,
+    ...(needsParam && !hasRules
       ? {
           parameterMapping: {
             parameterName: trimmedParamName,
             sourceField: resolvedSourceField,
           },
         }
       : {}),
-    ...(needsPage && actionRules.length === 0
+    ...(needsPage && !hasRules
       ? { targetPageId: trimmedTargetPageId }
       : {}),
     ...(chartType === "table" &&
     clickableColumns.length > 0 &&
-    actionRules.length === 0
+    !hasRules
       ? { clickableColumns }
       : {}),
-    ...(actionRules.length > 0 ? { rules: actionRules } : {}),
+    ...(hasRules ? { rules: actionRules } : {}),
   };
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/src/lib/widget-actions.ts` around lines 43 - 66, The current validation
block in widget-actions.ts (variables needsParam, needsPage, trimmedParamName,
trimmedSourceField, trimmedTargetPageId and checks referencing clickActionType
and layout) incorrectly rejects rule-based configs when actionRules is present;
change the logic in the function that computes the resolved action so that if
actionRules (or a non-empty actionRules array) exists you skip the legacy
top-level field guards (the needsParam/needsPage checks and the layout page-id
validation) and allow rule-only configurations to pass, while preserving the
existing validation path when actionRules is absent.

[coderabbitai]

🧹 Nitpick comments (1)

app/src/components/dashboard-container.tsx (1)

165-166: Use the resolved widget title for the download name.

The card header now comes from getWidgetDisplayTitle() + interpolateTitle(), but CSV export still names files from raw widget.settings.title. Template-backed and parameterized widgets can therefore download under a different title than the one shown in the UI.

♻️ Proposed fix

-    const title = (widget.settings?.title as string) || widget.chartType;
+    const title = interpolateTitle(getWidgetDisplayTitle(widget), parameters);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/src/components/dashboard-container.tsx` around lines 165 - 166, The CSV
export filename uses the raw widget.settings.title instead of the resolved
display title, so change the filename construction to use the computed title
from getWidgetDisplayTitle() and interpolateTitle(): call
getWidgetDisplayTitle(widget) (or the existing variable that holds that resolved
title) and pass its interpolated result into buildExportFilename instead of
widget.settings?.title; ensure any interpolation step (interpolateTitle()) is
applied before calling buildExportFilename so the download name matches the card
header.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@app/src/components/dashboard-container.tsx`:
- Around line 165-166: The CSV export filename uses the raw
widget.settings.title instead of the resolved display title, so change the
filename construction to use the computed title from getWidgetDisplayTitle() and
interpolateTitle(): call getWidgetDisplayTitle(widget) (or the existing variable
that holds that resolved title) and pass its interpolated result into
buildExportFilename instead of widget.settings?.title; ensure any interpolation
step (interpolateTitle()) is applied before calling buildExportFilename so the
download name matches the card header.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 39a93758-ba26-45b2-8da7-2bfb358dd663

📥 Commits

Reviewing files that changed from the base of the PR and between e67dc47 and 823c657.

📒 Files selected for processing (2)

• app/src/components/card-container.tsx
• app/src/components/dashboard-container.tsx

🚧 Files skipped from review as they are similar to previous changes (1)

• app/src/components/card-container.tsx

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

app/src/lib/__tests__/data-transforms.test.ts (1)

758-769: Type coercion behavior documented via test.

This test asserts that string "42" equals number 42. This is intentional loose equality per the implementation. Consider adding a brief comment in the test to clarify this is expected behavior, so future maintainers don't assume it's a bug.

Suggested clarification

     it("filter type coercion: string '42' == number 42", () => {
+      // Implementation uses loose equality to support mixed-type comparisons
+      // from heterogeneous data sources (e.g., CSV imports, JSON APIs)
       const data = [
         { id: "42", name: "match" },
         { id: "99", name: "no" },

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/src/lib/__tests__/data-transforms.test.ts` around lines 758 - 769, Add a
brief inline comment to the test that verifies filter type coercion to clarify
that the loose equality behavior is intentional: update the test around the
applyTransforms call (the test that uses Transform with operator "==" and value
42) to note that string "42" matching number 42 relies on intentional type
coercion in applyTransforms and is not a bug, so future maintainers understand
why this assertion is expected; reference the Transform type and the operator
"==" in the comment for clarity.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@app/e2e/transforms.spec.ts`:
- Around line 180-183: The test uses dialog.getAllByRole which is not a
Playwright API; update the selector in the removeButtons assignment to use
Playwright locators, e.g. replace dialog.getAllByRole("button", { name: "Remove
transform" }) with dialog.getByRole("button", { name: "Remove transform"
}).all() or use dialog.locator('role=button[name="Remove transform"]').all();
then keep the click call using .first().click() on the resulting Locator
(removeButtons variable) so the rest of the test (removeButtons.first().click())
works with Playwright locators.

---

Nitpick comments:
In `@app/src/lib/__tests__/data-transforms.test.ts`:
- Around line 758-769: Add a brief inline comment to the test that verifies
filter type coercion to clarify that the loose equality behavior is intentional:
update the test around the applyTransforms call (the test that uses Transform
with operator "==" and value 42) to note that string "42" matching number 42
relies on intentional type coercion in applyTransforms and is not a bug, so
future maintainers understand why this assertion is expected; reference the
Transform type and the operator "==" in the comment for clarity.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: cf78e9fa-f2db-40dc-8955-a3e4a229d6d2

📥 Commits

Reviewing files that changed from the base of the PR and between 823c657 and 5d85c51.

📒 Files selected for processing (2)

• app/e2e/transforms.spec.ts
• app/src/lib/__tests__/data-transforms.test.ts

[sonarqubecloud]

Quality Gate failed

Failed conditions
72.7% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud