Skip to content

AG-17719 Support Calculated Columns and Show Values As in AI Toolkit - #14329

Open
alistairjcbrown wants to merge 8 commits into
latestfrom
ag-17719/ai-toolkit-new-features
Open

AG-17719 Support Calculated Columns and Show Values As in AI Toolkit#14329
alistairjcbrown wants to merge 8 commits into
latestfrom
ag-17719/ai-toolkit-new-features

Conversation

@alistairjcbrown

@alistairjcbrown alistairjcbrown commented Jul 3, 2026

Copy link
Copy Markdown
Member

getStructuredSchema() only describes Grid State, so adding a column (a
column-definition change) could not be expressed and the LLM replied with
prose instead of acting. Extend both AI Toolkit examples' LLM response schema
with a columnDefOperations array and apply those operations via setGridOption
before setState. Enable calculatedColumns on both grids and steer the system
prompts to Calculated Column formula syntax, including quoting literal text.
The AI Toolkit examples call ai-api.ag-grid.com directly, which browsers block
from localhost on CORS (the upstream gates on an origin allow-list). Add a
dev-server-only Vite proxy at /ai-proxy that forwards upstream and can present
an allowed Origin/Referer and dev token server-side, both env-gated and inert
by default. Point an example's AI_API_URL at /ai-proxy via .env.local to use it.
This is dev tooling, separable from the example fix, and never ships in the
production static build.
showValuesAs was wired into ColumnState (getColumnState/applyColumnState) when
the feature landed, but never into the GridState projection, so getState/setState
silently dropped it. Carry it through AggregationColumnState, convertColumnState,
orderAggregationModel and the setState apply path so it round-trips like aggFunc.
Add a null schema builder (s.null) and extend the aggregation feature schema so
each aggregatable column exposes showValuesAs (built-in modes + null to clear)
when showValuesAsSvc is present. Behavioural tests cover the schema shape and the
full grid-state round-trip.
- Derive SHOW_VALUES_AS_BUILT_IN_TYPES from Record<ShowValuesAsBuiltInType, true>
  so the built-in mode set is compile-time coupled to the source union (typos,
  removals and additions now break the build instead of silently misinforming the LLM).
- Add a system-prompt note in both examples to carry over a column's existing
  showValuesAs on aggregation edits, since it is a required, nullable schema property.
- Cover the graceful-degradation branch: assert showValuesAs is omitted from the
  schema when ShowValuesAsModule is absent.
Comment thread packages/ag-grid-community/src/misc/state/stateService.ts
@github-actions

github-actions Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

❌ Codex review complete; 1 issue found (P0: 0 | P1: 1 | P2: 0 | P3: 0)

View full review

AG-17719 Support Calculated Columns and Show Values As in AI Toolkit

PR: #14329
Author: alistairjcbrown | Base: latest ← Head: ag-17719/ai-toolkit-new-features
Diff: 21 files changed, +792 -30

Summary

This PR extends AI Toolkit examples and structured schema support for calculated-column operations, and adds showValuesAs to aggregation grid state/schema round-tripping.

Findings

P0: 0 | P1: 1 | P2: 0 | P3: 0

1 inline comments posted.

Verdict

Assessment: incorrect
Confidence: 0.73

The feature direction is coherent, but the state restore path can preserve stale showValuesAs values when the new aggregation model omits the property, which is a substantive behavioural regression for setState replacement semantics.

Required Actions:

  • Clear showValuesAs for aggregation entries when the incoming model omits it, or otherwise preserve replacement semantics explicitly

@github-actions

github-actions Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Live-test this PR in Plunker

Paste these two <script> tags into a Plunker (or any vanilla-JS host) to load the UMD bundles built from this PR:

<script src="https://ag-grid.github.io/ag-grid/pr-14329/ag-grid-community.min.js"></script>
<script src="https://ag-grid.github.io/ag-grid/pr-14329/ag-grid-enterprise.min.js"></script>

Bundles are removed automatically when the PR is closed. Updated on every push.

@github-actions

github-actions Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Module Size Comparison

Extreme Values

🔺 Largest Increase: AllEnterpriseModule

  • Self Size: 1764.30 KB → 1764.94 KB (+0.64 KB, +0.0%)

Significant Changes (≥ 0.5KB)

Module(s) Base (KB) PR (KB) Diff (KB) Diff % Base Gzip (KB) PR Gzip (KB) Gzip Diff (KB) Gzip %
🔺 AllEnterpriseModule 1764.30 1764.94 +0.64 +0.0% 478.48 478.67 +0.19 +0.0%
📊 Full Statistics
  • Modules compared: 89
  • Modules with increases: 4
  • Modules with decreases: 0
  • Modules unchanged: 85

All Module Changes

Module(s) Base (KB) PR (KB) Diff (KB) Diff % Base Gzip (KB) PR Gzip (KB) Gzip Diff (KB) Gzip %
🔺 AllEnterpriseModule 1764.30 1764.94 +0.64 +0.0% 478.48 478.67 +0.19 +0.0%
🔺 AiToolkitModule 38.07 38.55 +0.48 +1.3% 12.32 12.48 +0.16 +1.3%
🔺 GridStateModule 16.93 17.08 +0.15 +0.9% 4.66 4.69 +0.03 +0.6%
🔺 AllCommunityModule 438.22 438.37 +0.15 +0.0% 119.33 119.38 +0.05 +0.0%

Updated: 2026-07-03T15:43:07.383Z

- Build the calculated-column ColDef by dropping the operation discriminator and
  unset values, instead of field-by-field conditional assignment.
- Keep the ChatToolPanel change minimal: add applyColumnDefOperations above the
  existing state-apply branch rather than restructuring the render/setState flow.
* this forwards it upstream, adding the dev token server-side if one is configured.
*/
proxy: {
'/ai-proxy': {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dev-only proxy: the examples call ai-api.ag-grid.com, which the browser CORS-blocks from localhost. Point an example's AI_API_URL at /ai-proxy via .env.local to test locally. vite.server.* isn't in the production build, so no prod footprint.

}
if (typeof aggFunc === 'string') {
aggregationColumns.push({ colId, aggFunc, valueIndex });
aggregationColumns.push({ colId, aggFunc, showValuesAs, valueIndex });

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

showValuesAs has lived in ColumnState since it shipped but was never wired into GridState — so getState() dropped it and setState() couldn't restore it. Follow-up AG-17765 planned which adds a guard against this (pivotSort is a second instance).

@AlpineJosh AlpineJosh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks good. A few minor points to clear up the schema for OpenAI.

I do think it might be worth us splitting up structured schema into individual tool schemas. That way we could provided the calculated column schema ourselves rather than requiring devs to manually type it.

Comment on lines +61 to +62
description:
'Calculated Column formula. Reference other columns with bracketed column IDs, for example [gold] * 3 + [silver] * 2 + [bronze], and wrap literal text in double quotes, for example "Hello"',

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may need more detail here as to valid operators and functions. It can also be provided as top level context.

description: 'Whether the new column should be hidden',
},
},
required: ['operation', 'colId', 'headerName', 'calculatedExpression'],

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Annoyingly ChatGPT requires that all fields are marked as required. The only way to make them optional is to union with 'null'. See https://developers.openai.com/api/docs/guides/structured-outputs#supported-schemas

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants