feat(cel): custom data transformation functions (#14)#21
Merged
michaelmcnees merged 21 commits intomainfrom Mar 25, 2026
Merged
feat(cel): custom data transformation functions (#14)#21michaelmcnees merged 21 commits intomainfrom
michaelmcnees merged 21 commits intomainfrom
Conversation
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Registers custom string member overloads via cel.Lib and wires them into NewEvaluator through a central customFunctions() registration point. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Covers three patterns — CEL-only structural transforms, AI-powered semantic transforms, and hybrid workflows that combine both. Includes a decision guide and a quick-reference table of all available CEL extension functions. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ests (#14) - default(): fall through to rhs when lhs is null (types.NullValue) - parseTimestamp(): try RFC3339Nano, bare ISO date, US date, and named-month formats before erroring - expressions.md: remove undeclared `now` variable from two examples - data-transformations.md: add missing exists_one row to list-macros table - data-transform-api-to-db.yaml: simplify to single-user fetch+insert (fixes broken batch param shape) - functions_test.go: add null/fallback tests for default(), edge-case tests for flatten(), and new format tests for parseTimestamp() Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThe PR adds comprehensive CEL custom functions for string manipulation, type coercion, collections, JSON encoding/decoding, and timestamp parsing/formatting. It includes workflow examples demonstrating AI-powered and standard data transformation patterns, extensive test coverage, and documentation. Previous planning documents for connection recovery were removed. Changes
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~35 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 12
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/superpowers/plans/2026-03-24-data-transformation.md`:
- Line 15: The task headings use H3 (e.g., "### Task 1: Test and document
built-in macros") which skips from the document H1 and breaks the outline;
change those "### Task ..." headings to H2 ("## Task ...") and update any
subsequent task sections that repeat the H3 pattern so all task headings
consistently use "##" to satisfy markdownlint and preserve proper heading
hierarchy.
- Around line 689-742: The plan's
collectionFunctions()/collectionLib.CompileOptions() currently describes a
single variadic obj overload (nil arg types), but cel-go requires fixed-arity
overloads; update collectionLib.CompileOptions() to register individual
cel.Function overloads for obj with explicit arg type slices for 2, 4, 6, 8 and
10 parameters (e.g., []ref.Type{cel.DynType, cel.DynType} etc.), reusing the
same binding logic (validate even arg count and string keys, call refToNative on
values) in each overload's FunctionBinding, and ensure customFunctions() still
includes collectionFunctions(); keep error messages and behavior identical.
- Around line 1018-1130: The plan implements a function named timestamp which
conflicts with CEL's built-in timestamp(string); rename the custom symbol to
parseTimestamp everywhere: update the cel.Function call and overload name in
timeLib (e.g., change "timestamp"/"timestamp_string" to
"parseTimestamp"/"parseTimestamp_string"), update any binding logic references
in timeFunctions/timeLib and the formatTimestamp overload that expects the
parsed timestamp (keep formatTimestamp name), and update tests
(TestFunc_Timestamp -> TestFunc_ParseTimestamp or at least change the
expressions to use parseTimestamp(...)) and any references in customFunctions
where timeFunctions is added; ensure imports (time) remain and bindings still
return types.Timestamp for the parse function.
In `@docs/superpowers/specs/2026-03-24-data-transformation-design.md`:
- Around line 70-75: The spec uses the wrong function name: replace
`timestamp(string)` with `parseTimestamp(string)` in the table and any examples
so it matches the implementation and other docs (e.g., expressions.md and
functions_test.go); update the example line to
`parseTimestamp("2026-03-24T19:00:00Z")` and ensure the description still notes
Go time layout strings and the `formatTimestamp(ts, layout)` example remains
unchanged.
- Around line 87-99: The code example for NewEvaluator is inaccurate: instead of
inlining customFunctions()... into cel.NewEnv, the real implementation builds a
slice of cel.EnvOption first (opts := []cel.EnvOption{...}) and then appends the
custom functions via opts = append(opts, customFunctions()...) before calling
cel.NewEnv(opts...). Update the snippet to show the opts variable being
initialized with the cel.Variable(...) entries, appending customFunctions() to
opts, and then passing opts to cel.NewEnv so it matches the actual
implementation (referencing NewEvaluator, opts, customFunctions, and
cel.NewEnv).
In `@examples/ai-data-enrichment.yaml`:
- Around line 57-65: The YAML for the postgres/query step uses the wrong key
name — replace the second params: block (the list of query arguments under the
INSERT INTO urgent_tickets ... statement) with args: so the connector receives
arguments under args; keep the same templated values (e.g., "{{
steps.classify.output.json.priority }}", "{{
jsonEncode(steps.classify.output.json.products) }}", "{{
steps['fetch-tickets'].output.body }}") and only change the key name from params
to args to match the postgres/query contract.
In `@examples/data-transform-api-to-db.yaml`:
- Around line 2-6: Update the description to match the actual CEL usage or add
an `obj()` example in the workflow: either remove `obj()` (and possibly `string
functions`) from the front-matter description and state that it demonstrates
`toLower()` only, or modify the CEL transform expressions to include a concrete
`obj()` call (e.g., using `obj(someJson).field` in the transformation) alongside
`toLower()` so the description "Demonstrates obj(), toLower(), and string
functions" is accurate; adjust the wording around `string functions` too if you
only keep `toLower()`.
In `@internal/cel/functions_test.go`:
- Around line 201-242: Add a test in TestFunc_Obj to cover the spec case where
obj() is called with a non-string key and should return an error: update the
tests slice in TestFunc_Obj (the table used by NewEvaluator() and eval.Eval with
newTestContext()) to include a case like name: "non_string_key", expr: `obj(1,
"value")` (or similar non-string key), and set wantErr: true so the test asserts
an error is returned; keep the rest of the test harness (Eval call and
assertions) unchanged so the new case follows the same require.Error check.
In `@internal/cel/functions.go`:
- Around line 270-281: The jsonDecode overload currently unmarshals into an any
which turns all JSON numbers into float64 and loses integer precision; fix
jsonDecode by using json.Decoder with UseNumber() to parse numbers as
json.Number and then implement and call a helper (e.g., normalizeJSONNumbers) to
walk the parsed structure and convert json.Number values into appropriate
numeric types (int64 when the string represents an integer within range,
otherwise float64 or a high-precision representation), returning the normalized
value to types.DefaultTypeAdapter.NativeToValue; ensure the helper handles
nested maps and slices and is referenced from the UnaryBinding in the
cel.Function("jsonDecode") overload.
- Around line 157-168: The overload "default_any_any" is currently strict so CEL
will propagate error/unknown before the BinaryBinding runs, making the lhs
checks ineffective; change the overload to be non-strict by adding the CEL
non-strict option to the overload declaration (e.g., use cel.NonStrict or the
library's non-strict overload option) for the Overload("default_any_any", ...)
so the BinaryBinding for default handles error/unknown lhs and returns rhs when
lhs is error/unknown/null.
In `@site/src/content/docs/concepts/expressions.md`:
- Around line 490-503: Update the `obj()` documentation to state that, due to
cel-go fixed-arity overloads, `obj()` supports only up to 5 key-value pairs
(i.e., 2/4/6/8/10 arguments) rather than an unlimited number; mention callers
will get an error if they pass more than 10 arguments and suggest using nested
objects or helper functions for larger maps. Reference the `obj()` expression
name in the text so readers can find the exact function.
In `@site/src/content/docs/getting-started/data-transformations.md`:
- Around line 266-274: The table lists string transformations as global
functions (toLower(s), toUpper(s), trim(s), replace(s, old, new), split(s, sep))
but the implementation and expressions.md use string methods; update each table
row to show method syntax (e.g., s.toLower() / "hello".toLower(), s.toUpper() /
"HELLO".toUpper(), s.trim() / " a ".trim(), s.replace(old, new) /
"a-b".replace("-", "_"), s.split(sep) / "a,b".split(",")) and adjust the
Description column if needed so the table matches the actual method-based API in
expressions.md and the codebase.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: eba3db8a-214b-484b-9f64-8bca2bcbd3a9
📒 Files selected for processing (10)
docs/superpowers/plans/2026-03-24-data-transformation.mddocs/superpowers/specs/2026-03-24-data-transformation-design.mdexamples/ai-data-enrichment.yamlexamples/data-transform-api-to-db.yamlinternal/cel/cel.gointernal/cel/functions.gointernal/cel/functions_test.gointernal/cel/macros_test.gosite/src/content/docs/concepts/expressions.mdsite/src/content/docs/getting-started/data-transformations.md
docs/superpowers/specs/2026-03-24-data-transformation-design.md
Outdated
Show resolved
Hide resolved
docs/superpowers/specs/2026-03-24-data-transformation-design.md
Outdated
Show resolved
Hide resolved
…, docs accuracy Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Deploying mantle with
|
| Latest commit: |
ed5845c
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://6c4d32f5.mantle-cxy.pages.dev |
| Branch Preview URL: | https://feat-data-transformation-cel.mantle-cxy.pages.dev |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/superpowers/plans/2026-03-24-data-transformation.md`:
- Around line 821-831: The plan still contains older, pre-fix implementations
for cel.Function("default"), jsonDecode(), and parseTimestamp(); update the
document snippets to match the shipped implementations in
internal/cel/functions.go: replace the strict/null-blind default overload with
the current null-coalescing/error-aware implementation used in production, swap
the jsonDecode example that uses json.Unmarshal with the actual decoder
wrapper/panic-safe implementation, and expand parseTimestamp examples to include
all accepted formats (not only RFC3339) as in the shipped parseTimestamp
function; reference the function names cel.Function("default"), jsonDecode, and
parseTimestamp when making these replacements so the docs reflect the real code.
- Around line 1279-1282: The Postgres examples use a nested params key and a
single $1,$2,$3 tuple while mapping produces multiple rows; update both Task 11
snippets to use the connector's args parameter (not nested params) and supply an
array-of-rows that matches the three bindings per row: set args to something
like steps['fetch-users'].output.json.map(u => [u.username.toLower(),
u.email.toLower(), u.address.city]) so the postgres/query call receives args
(array of arrays) and the INSERT statement (INSERT INTO users (username, email,
city) VALUES ($1, $2, $3)) will be executed with each inner array as bindings;
apply the same change to the other occurrence referenced (lines ~1345-1353).
In `@internal/cel/functions_test.go`:
- Around line 252-264: Add a regression test to TestFunc_Default to exercise the
non-strict missing-key overload of default(): add a test case entry in the tests
slice (inside TestFunc_Default) that uses an unresolved/missing path expression
such as `default(steps.fetch.output.missing, "fallback")` and expects
"fallback"; ensure the new case name clearly indicates it's the
missing-field/non-strict overload so it will catch regressions to default's
missing-key behavior.
In `@internal/cel/functions.go`:
- Around line 277-283: After decoding into result with dec.Decode, ensure there
is no trailing JSON by checking the decoder for extra data (e.g. use dec.More()
or attempt to read the next token and expect io.EOF); if extra data exists
return an error (using types.NewErr) so inputs like "{} {}" are rejected. Update
the block around dec, result, normalizeJSONNumbers and
types.DefaultTypeAdapter.NativeToValue to perform this EOF/trailing-data check
and return an explicit jsonDecode error when trailing content is found.
In `@site/src/content/docs/getting-started/data-transformations.md`:
- Around line 87-92: The examples for the ai/completion step are inconsistent:
the snippet for the classify step places credential under params while the
runnable example (examples/ai-data-enrichment.yaml) uses a step-level
credential; update the classify step (and the other similar snippet referenced)
to move credential out of params to the same step-level field (e.g., keep name:
classify and action: ai/completion, remove credential from params and add
credential: openai at the step root) so all ai/completion examples use the same
shape and avoid confusing copy/paste users.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: be4230a6-821d-46ba-b484-f68d4fc1eb73
📒 Files selected for processing (8)
docs/superpowers/plans/2026-03-24-data-transformation.mddocs/superpowers/specs/2026-03-24-data-transformation-design.mdexamples/ai-data-enrichment.yamlexamples/data-transform-api-to-db.yamlinternal/cel/functions.gointernal/cel/functions_test.gosite/src/content/docs/concepts/expressions.mdsite/src/content/docs/getting-started/data-transformations.md
…cs accuracy Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
internal/cel/functions.go (1)
277-287:⚠️ Potential issue | 🟠 Major
dec.More()is still too weak for top-level trailing-data checks.After a successful top-level
Decode,Decoder.More()only peeks for]/}. Inputs likejsonDecode("{}]")orjsonDecode("1}")can still slip through as success. Use a real EOF check (Token()or a secondDecode()expectingio.EOF) and add one of those cases to the regression suite.📝 Suggested fix
import ( "encoding/json" "fmt" + "io" "strconv" "strings" "time" @@ - // Reject trailing data (e.g., "{} {}") - if dec.More() { - return types.NewErr("jsonDecode: unexpected trailing data after JSON value") - } + if _, err := dec.Token(); err != io.EOF { + if err != nil { + return types.NewErr("jsonDecode: %v", err) + } + return types.NewErr("jsonDecode: unexpected trailing data after JSON value") + } return types.DefaultTypeAdapter.NativeToValue(normalizeJSONNumbers(result))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/cel/functions.go` around lines 277 - 287, The current JSON trailing-data check using dec.More() is insufficient; after Decode succeeds you must verify there is no non-whitespace trailing data by ensuring the decoder hits EOF (e.g., call dec.Token() and allow only io.EOF or perform a second dec.Decode expecting io.EOF) instead of relying on dec.More(); update the code around the JSON decoding block (the decoder created in the json.NewDecoder(strings.NewReader(s)) path that returns types.DefaultTypeAdapter.NativeToValue(normalizeJSONNumbers(result))) to perform the EOF check and return a jsonDecode-style error when extra data is found, and add regression tests for cases like "{}]" and "1}" to the test suite.docs/superpowers/plans/2026-03-24-data-transformation.md (1)
1073-1076:⚠️ Potential issue | 🟡 MinorThere are still stale
timestamp()references in the plan.Line 1076 says the tests fail because the runtime registers
parseTimestamp(), even though the test expressions above already callparseTimestamp(). Line 1206 still tells docs writers to publishtimestamp(string). Leaving both behind reintroduces the old name the implementation intentionally avoided to prevent a collision with CEL's built-intimestamp().📝 Suggested edits
-Expected: FAIL (functions registered as `parseTimestamp`, not `timestamp`) +Expected: FAIL (functions not registered yet) ... -- `timestamp(string)`, `formatTimestamp(ts, layout)` with Go layout reference +- `parseTimestamp(string)`, `formatTimestamp(ts, layout)` with Go layout referenceAlso applies to: 1205-1206
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/superpowers/plans/2026-03-24-data-transformation.md` around lines 1073 - 1076, Remove the stale references to timestamp() in the plan and update the narrative to consistently use the runtime-registered parseTimestamp() name: change the test expectation text around TestFunc_Timestamp and TestFunc_FormatTimestamp to state that functions are registered as parseTimestamp (not timestamp), and update any guidance to docs writers (previously instructing to publish timestamp(string)) to instead instruct not to publish the old timestamp name and to use parseTimestamp(string) to avoid colliding with CEL's built-in timestamp(); ensure all occurrences of "timestamp()" in the document are replaced or clarified to "parseTimestamp()" so the plan and publishing guidance are consistent.site/src/content/docs/getting-started/data-transformations.md (1)
135-138:⚠️ Potential issue | 🟠 MajorDon't promise engine-side schema validation.
Line 137 attributes schema validation to Mantle itself, but the code only forwards
output_schemato the provider. OpenAI gets native enforcement, while Bedrock only gets a prompt instruction, so “schema-validated object” is not a universal engine guarantee.📝 Suggested wording
-- `output_schema` tells the AI connector to return structured JSON matching the schema, not free-form text. The engine validates the response against the schema before making it available as `output.json`. -- The `if` field on `store-escalation` is a bare CEL expression that reads from the AI step's structured output. CEL works on the schema-validated object directly. +- `output_schema` tells the AI connector to request structured JSON instead of free-form text. Provider behavior varies: some providers enforce the schema natively, while others treat it as guidance only. +- The `if` field on `store-escalation` is a bare CEL expression that reads from the AI step's parsed JSON output.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/superpowers/plans/2026-03-24-data-transformation.md`:
- Around line 1386-1389: Update the final validation checklist to include
running golangci-lint in addition to the existing "Step 2: Run go vet" item:
replace or augment the "Run: `go vet ./internal/cel/` Expected: clean" entry
with an added step to run `golangci-lint run ./...` (or the repository's
configured golangci-lint command) and mark the expected result as clean so local
validation matches CI linting requirements.
In `@site/src/content/docs/getting-started/data-transformations.md`:
- Around line 295-300: Update the documentation entry for parseTimestamp(s) to
reflect the actual accepted input formats: note that internal/cel/functions.go
accepts RFC3339 and RFC3339Nano, bare ISO datetimes, date-only strings, US-style
dates, and named-month formats (not just RFC3339). Edit the table row for
`parseTimestamp(s)` so the Description lists these formats concisely and
references the parsing behavior in internal/cel/functions.go (e.g., “Accepts
RFC3339/RFC3339Nano, bare ISO datetimes, date-only strings, US dates, and
named-month formats”).
---
Duplicate comments:
In `@docs/superpowers/plans/2026-03-24-data-transformation.md`:
- Around line 1073-1076: Remove the stale references to timestamp() in the plan
and update the narrative to consistently use the runtime-registered
parseTimestamp() name: change the test expectation text around
TestFunc_Timestamp and TestFunc_FormatTimestamp to state that functions are
registered as parseTimestamp (not timestamp), and update any guidance to docs
writers (previously instructing to publish timestamp(string)) to instead
instruct not to publish the old timestamp name and to use parseTimestamp(string)
to avoid colliding with CEL's built-in timestamp(); ensure all occurrences of
"timestamp()" in the document are replaced or clarified to "parseTimestamp()" so
the plan and publishing guidance are consistent.
In `@internal/cel/functions.go`:
- Around line 277-287: The current JSON trailing-data check using dec.More() is
insufficient; after Decode succeeds you must verify there is no non-whitespace
trailing data by ensuring the decoder hits EOF (e.g., call dec.Token() and allow
only io.EOF or perform a second dec.Decode expecting io.EOF) instead of relying
on dec.More(); update the code around the JSON decoding block (the decoder
created in the json.NewDecoder(strings.NewReader(s)) path that returns
types.DefaultTypeAdapter.NativeToValue(normalizeJSONNumbers(result))) to perform
the EOF check and return a jsonDecode-style error when extra data is found, and
add regression tests for cases like "{}]" and "1}" to the test suite.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: ab28b0a4-5a72-4679-8f98-697538f3b6cf
📒 Files selected for processing (4)
docs/superpowers/plans/2026-03-24-data-transformation.mdinternal/cel/functions.gointernal/cel/functions_test.gosite/src/content/docs/getting-started/data-transformations.md
…leanup
- jsonDecode: replace dec.More() with dec.Decode(&trailing) EOF check
to catch all trailing data (e.g., "{}]", "1}")
- Add trailing_bracket and trailing_brace regression tests
- Update parseTimestamp description in data-transformations.md to list
all accepted formats
- Plan: rename TestFunc_Timestamp to TestFunc_ParseTimestamp, add
golangci-lint to final validation checklist
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (5)
docs/superpowers/plans/2026-03-24-data-transformation.md (3)
1205-1207:⚠️ Potential issue | 🟠 MajorTask 9 still asks the writer to document
timestamp().The custom helper shipped in this PR is
parseTimestamp(). Leavingtimestamp(string)here points the docs back at CEL’s built-in parser instead of the extension you actually add.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/superpowers/plans/2026-03-24-data-transformation.md` around lines 1205 - 1207, Replace the incorrect reference to the built-in timestamp(string) with the actual custom helper parseTimestamp(string) in the Date/Time Functions docs (and any places Task 9 references timestamp()); update the entry to list parseTimestamp(string) and describe its behavior, parameters and that it supersedes CEL’s built-in parser, and keep formatTimestamp(ts, layout) with the Go layout reference unchanged so readers use parseTimestamp() not timestamp().
704-723:⚠️ Potential issue | 🟠 MajorThe
obj()task still shows the old variadic overload.The note above correctly says the implementation uses fixed 2/4/6/8/10-argument overloads, but this snippet still registers a single
nil-typed overload. Following it will diverge frominternal/cel/functions.go.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/superpowers/plans/2026-03-24-data-transformation.md` around lines 704 - 723, The doc still shows a single variadic overload for cel.Function("obj") but the implementation uses explicit fixed-arity overloads; replace the nil (variadic) overload with explicit cel.Overload entries for the supported arities (2,4,6,8,10) each returning cel.DynType and using the same FunctionBinding logic (validate even args, check keys are types.String, call refToNative for values, return types.DefaultTypeAdapter.NativeToValue(m), and produce errors via types.NewErr). Ensure each overload signature matches the fixed number of ref.Val parameters so the docs mirror internal/cel/functions.go.
978-985:⚠️ Potential issue | 🟠 MajorKeep the
jsonDecode()plan snippet in sync with the trailing-data fix.This example returns after the first
Decode, so inputs like"{} {}"are effectively documented as valid again. The shipped code now performs a second decode and expectsio.EOF.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/superpowers/plans/2026-03-24-data-transformation.md` around lines 978 - 985, The jsonDecode plan currently returns immediately after the first dec.Decode(&result), which allows trailing JSON to be accepted; after successfully decoding into result in the jsonDecode snippet, perform a second decode into a temporary variable (e.g., var extra any) and ensure the decoder returns io.EOF; if the second Decode returns nil or any error other than io.EOF, return a types.NewErr indicating trailing data (or the decode error) instead of returning the parsed value, then continue to return types.DefaultTypeAdapter.NativeToValue(normalizeJSONNumbers(result)) upon EOF. Make sure to reference the json decoder (dec), result variable, normalizeJSONNumbers, and types.DefaultTypeAdapter.NativeToValue in the fix and handle io.EOF comparison.site/src/content/docs/getting-started/data-transformations.md (1)
274-279:⚠️ Potential issue | 🟡 MinorExpose
obj()'s 5-pair limit in the reference table.
obj(key, val, ...)reads like a true variadic helper, butinternal/cel/functions.goonly registers 2/4/6/8/10-argument overloads. Without the cap here, readers can copy an expression that fails in the evaluator once they go past five pairs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@site/src/content/docs/getting-started/data-transformations.md` around lines 274 - 279, The table entry for obj(key, val, ...) is misleadingly variadic; update the docs to state the actual limit (up to 5 key/value pairs) or list supported overloads explicitly. Mention the specific helper name obj() and reference the overload registrations in internal/cel/functions.go (2/4/6/8/10-argument overloads) so readers know the evaluator caps at five pairs. Keep the description short and replace or augment "obj(key, val, ...)" with "obj(key, val, ... up to 5 pairs)" or enumerated signatures.internal/cel/functions.go (1)
353-362:⚠️ Potential issue | 🟠 Major
jsonDecode()still rounds integers that don't fit inint64.A pure integer like
9223372036854775808falls through toFloat64()here, so the decoded value loses bits silently. Preserve out-of-range integer tokens exactly instead of coercing them tofloat64, and add a regression test for that path.Preserve out-of-range integers exactly
func normalizeJSONNumbers(v any) any { switch val := v.(type) { case json.Number: - if i, err := val.Int64(); err == nil { + s := val.String() + if !strings.ContainsAny(s, ".eE") { + if i, err := val.Int64(); err == nil { + return i + } + return s + } + if f, err := val.Float64(); err == nil { + return f + } + return s - return i - } - if f, err := val.Float64(); err == nil { - return f - } - return val.String() case map[string]any:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/cel/functions.go` around lines 353 - 362, normalizeJSONNumbers currently falls back to Float64 when Int64 fails, which silently loses precision for integers > int64; change the logic in normalizeJSONNumbers (and update jsonDecode tests) so that if json.Number.String() contains no decimal point or exponent and val.Int64() returns an error, you preserve the exact token by returning val.String() (not val.Float64()); only use Float64 for numbers that include a decimal point or an exponent. Add a regression test that jsonDecode returns the exact string for a large integer like "9223372036854775808" (and assert it is not converted to a float).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/superpowers/plans/2026-03-24-data-transformation.md`:
- Around line 419-424: Remove the unused local `result` and its population loop
from the split() snippet: delete the lines that create `result :=
make([]ref.Val, len(parts))` and the for-loop that assigns `result[i] =
types.String(p)`, leaving the function to directly return
types.DefaultTypeAdapter.NativeToValue(parts); this removes the dead-variable
compile error when copying into functions.go while keeping the intended return
value.
In `@internal/cel/functions_test.go`:
- Around line 312-324: The test "empty list" currently allows
flatten(inputs.empty) to return nil; change it to require a list instead: after
calling eval.Eval(`flatten(inputs.empty)`, ctx) assert no error, then assert
that result is a []any (use a type assertion on result and require.True or
require.IsType) and finally assert that the resulting slice is empty; reference
the test block named "empty list", the call to eval.Eval and the flattened
function `flatten(inputs.empty)` so the assertion enforces a non-nil []any
return value.
---
Duplicate comments:
In `@docs/superpowers/plans/2026-03-24-data-transformation.md`:
- Around line 1205-1207: Replace the incorrect reference to the built-in
timestamp(string) with the actual custom helper parseTimestamp(string) in the
Date/Time Functions docs (and any places Task 9 references timestamp()); update
the entry to list parseTimestamp(string) and describe its behavior, parameters
and that it supersedes CEL’s built-in parser, and keep formatTimestamp(ts,
layout) with the Go layout reference unchanged so readers use parseTimestamp()
not timestamp().
- Around line 704-723: The doc still shows a single variadic overload for
cel.Function("obj") but the implementation uses explicit fixed-arity overloads;
replace the nil (variadic) overload with explicit cel.Overload entries for the
supported arities (2,4,6,8,10) each returning cel.DynType and using the same
FunctionBinding logic (validate even args, check keys are types.String, call
refToNative for values, return types.DefaultTypeAdapter.NativeToValue(m), and
produce errors via types.NewErr). Ensure each overload signature matches the
fixed number of ref.Val parameters so the docs mirror internal/cel/functions.go.
- Around line 978-985: The jsonDecode plan currently returns immediately after
the first dec.Decode(&result), which allows trailing JSON to be accepted; after
successfully decoding into result in the jsonDecode snippet, perform a second
decode into a temporary variable (e.g., var extra any) and ensure the decoder
returns io.EOF; if the second Decode returns nil or any error other than io.EOF,
return a types.NewErr indicating trailing data (or the decode error) instead of
returning the parsed value, then continue to return
types.DefaultTypeAdapter.NativeToValue(normalizeJSONNumbers(result)) upon EOF.
Make sure to reference the json decoder (dec), result variable,
normalizeJSONNumbers, and types.DefaultTypeAdapter.NativeToValue in the fix and
handle io.EOF comparison.
In `@internal/cel/functions.go`:
- Around line 353-362: normalizeJSONNumbers currently falls back to Float64 when
Int64 fails, which silently loses precision for integers > int64; change the
logic in normalizeJSONNumbers (and update jsonDecode tests) so that if
json.Number.String() contains no decimal point or exponent and val.Int64()
returns an error, you preserve the exact token by returning val.String() (not
val.Float64()); only use Float64 for numbers that include a decimal point or an
exponent. Add a regression test that jsonDecode returns the exact string for a
large integer like "9223372036854775808" (and assert it is not converted to a
float).
In `@site/src/content/docs/getting-started/data-transformations.md`:
- Around line 274-279: The table entry for obj(key, val, ...) is misleadingly
variadic; update the docs to state the actual limit (up to 5 key/value pairs) or
list supported overloads explicitly. Mention the specific helper name obj() and
reference the overload registrations in internal/cel/functions.go
(2/4/6/8/10-argument overloads) so readers know the evaluator caps at five
pairs. Keep the description short and replace or augment "obj(key, val, ...)"
with "obj(key, val, ... up to 5 pairs)" or enumerated signatures.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e0e79ecd-e6f2-4ca1-88b0-0b06aad16b87
📒 Files selected for processing (4)
docs/superpowers/plans/2026-03-24-data-transformation.mdinternal/cel/functions.gointernal/cel/functions_test.gosite/src/content/docs/getting-started/data-transformations.md
- normalizeJSONNumbers: only use float64 for decimals/exponents, preserve overflow integers as strings to avoid silent precision loss - flatten: initialize result as make([]any, 0) so empty lists return non-nil empty slice - Add large_integer_preserved test for jsonDecode - Plan: remove dead split() variable, replace variadic obj() with fixed-arity overloads, add jsonDecode EOF check, fix timestamp refs - data-transformations.md: note obj() 5-pair limit Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/superpowers/plans/2026-03-24-data-transformation.md`:
- Around line 1148-1151: The test command uses the wrong selector and will skip
the ParseTimestamp test; update the test run invocation to include the correct
test name TestFunc_ParseTimestamp (replace or augment the current selector
TestFunc_Timestamp|TestFunc_FormatTimestamp with TestFunc_ParseTimestamp, e.g.
TestFunc_ParseTimestamp|TestFunc_Timestamp|TestFunc_FormatTimestamp) so the
parse test defined as TestFunc_ParseTimestamp is executed when running go test
./internal/cel/ -run "...".
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 59b8c48a-f403-4859-9620-5fe78aebc70a
📒 Files selected for processing (4)
docs/superpowers/plans/2026-03-24-data-transformation.mdinternal/cel/functions.gointernal/cel/functions_test.gosite/src/content/docs/getting-started/data-transformations.md
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
These internal process docs (specs, plans) are already in .gitignore but were force-added during development. Files remain on disk but are no longer tracked by git. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
|
Note Unit test generation is a public access feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
Contributor
|
✅ Unit tests committed locally. Commit: |
Contributor
|
Note Unit test generation is a public access feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
Contributor
|
✅ Created PR with unit tests: #26 |
7 tasks
michaelmcnees
added a commit
that referenced
this pull request
Mar 25, 2026
* docs: add spec for data transformation CEL functions (#14) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * docs: add implementation plan for data transformation CEL functions (#14) 12-task plan: macro tests, string/type/collection/JSON/time functions, obj() construction, docs updates, and example workflows. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * test(cel): add coverage for built-in map/filter/exists/all macros * feat(cel): add toLower, toUpper, trim string functions Registers custom string member overloads via cel.Lib and wires them into NewEvaluator through a central customFunctions() registration point. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(cel): add replace and split string functions * feat(cel): add parseInt, parseFloat, toString type coercion functions * feat(cel): add obj() map construction function * feat(cel): add default() null coalescing and flatten() functions * feat(cel): add jsonEncode and jsonDecode functions * feat(cel): add timestamp and formatTimestamp date/time functions * docs: add custom CEL functions and macros to expressions reference (#14) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat: add data transformation and AI enrichment example workflows (#14) * docs: add data transformation patterns guide (#14) Covers three patterns — CEL-only structural transforms, AI-powered semantic transforms, and hybrid workflows that combine both. Includes a decision guide and a quick-reference table of all available CEL extension functions. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: address CodeRabbit review — null handling, date formats, docs, tests (#14) - default(): fall through to rhs when lhs is null (types.NullValue) - parseTimestamp(): try RFC3339Nano, bare ISO date, US date, and named-month formats before erroring - expressions.md: remove undeclared `now` variable from two examples - data-transformations.md: add missing exists_one row to list-macros table - data-transform-api-to-db.yaml: simplify to single-user fetch+insert (fixes broken batch param shape) - functions_test.go: add null/fallback tests for default(), edge-case tests for flatten(), and new format tests for parseTimestamp() Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: address PR #21 review — non-strict default, jsonDecode precision, docs accuracy Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: address PR #21 review round 3 — test coverage, trailing JSON, docs accuracy Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: address PR #21 review round 4 — EOF trailing check, docs, plan cleanup - jsonDecode: replace dec.More() with dec.Decode(&trailing) EOF check to catch all trailing data (e.g., "{}]", "1}") - Add trailing_bracket and trailing_brace regression tests - Update parseTimestamp description in data-transformations.md to list all accepted formats - Plan: rename TestFunc_Timestamp to TestFunc_ParseTimestamp, add golangci-lint to final validation checklist Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: address PR #21 review round 5 — precision, flatten, plan accuracy - normalizeJSONNumbers: only use float64 for decimals/exponents, preserve overflow integers as strings to avoid silent precision loss - flatten: initialize result as make([]any, 0) so empty lists return non-nil empty slice - Add large_integer_preserved test for jsonDecode - Plan: remove dead split() variable, replace variadic obj() with fixed-arity overloads, add jsonDecode EOF check, fix timestamp refs - data-transformations.md: note obj() 5-pair limit Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(plan): correct test selector to TestFunc_ParseTimestamp Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * chore: remove force-added superpowers docs from tracking These internal process docs (specs, plans) are already in .gitignore but were force-added during development. Files remain on disk but are no longer tracked by git. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * 📝 CodeRabbit Chat: Add generated unit tests * feat(site): add Docker connector to homepage, fix tablet layout issues - Add Docker connector card to connectors grid, update count to 8/10 - Update hero section connector count - Add Docker to built-in connector list in plugins guide - Fix bottom nav visible on tablets where top nav already shows (lg:hidden → md:hidden) - Fix nav link overlap at tablet sizes (gap-8 → gap-4 lg:gap-8) - Delay Get Started button to md breakpoint with tighter padding - Sync bottom padding and sidebar offset to md breakpoint Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat: add execution_artifacts table (migration 013) * feat: add ArtifactDecl and RegistryCredential to Step struct - Add ArtifactDecl type to declare files produced by a step - Add ArtifactRef type for runtime representation in CEL expressions - Add registry_credential field to Step for container registry auth - Add artifacts field to Step for artifact declarations - Add test case for parsing artifacts and registry credentials Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat: add docker credential type (all fields optional) * feat: validate artifact declarations (unique names, required fields) * feat: add TmpStorage interface with filesystem backend and artifact context helpers Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat: add artifacts variable to CEL environment Add support for accessing artifact metadata in CEL expressions: - Add Artifacts field to Context struct for {name, url, size} - Register artifacts variable in CEL environment as a map - Initialize artifacts to empty map if nil during evaluation - Add test for accessing artifact URL and size properties Workflow authors can now reference artifacts via artifacts['name'].url in step parameters, enabling dynamic artifact references. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat: add artifact metadata store with Postgres backend Implements Store with Create, GetByName, ListByExecution, DeleteByExecution, and ListExpired operations backed by the execution_artifacts table (migration 013). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat: add docker/run param parsing and types Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat: add artifact reaper for TTL-based cleanup Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * docs: add Docker volume backup and data transform example workflows Refs #12, refs #13 * docs: add docker/run connector reference documentation * feat: implement docker/run connector with Moby client Add Execute method to DockerRunConnector using the Moby Go client, supporting image pull policies, stdin, env vars, volume mounts, artifacts dir bind-mount, resource limits, context cancellation, and stdout/stderr capture with 10MB limit per stream. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat: wire artifact lifecycle and registry credential into engine Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: address CodeRabbit review findings - Handle filepath.Abs errors in FilesystemTmpStorage.DeleteByPrefix - Fix artifact reaper over-counting on partial file deletion failure - Extract loadArtifactsIntoCELContext helper, log errors instead of silencing - Return error on invalid memory limit in docker/run - Log stdcopy.StdCopy errors in docker/run - Dynamic credential type list in GetType error message - Add missing error checks in tests - Fix idiomatic boolean check in docker_test.go Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: address CodeRabbit review findings (site/examples) - Fix Hero.astro connector count to match Connectors.astro (8 not 10) - Make volume backup notification conditions mutually exclusive Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: update example workflow count in Hero (17 → 22) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * docs: add Docker workflows guide, artifacts concept page, and config/credential docs - Add getting-started/docker-workflows.md with examples for stdin, env, resource limits, exit code branching, private images, volume mounts - Add concepts/artifacts.md explaining the artifact system, tmp storage, CEL access, retention, and scope - Add docker and aws credential types to secrets-guide.md - Add tmp storage config fields to configuration.md (config table + env vars) - Add Data Transformations, Docker Workflows, and Artifacts to docs nav Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: address CI failures — deprecated ImageInspectWithRaw, gosec G118 - Replace deprecated ImageInspectWithRaw with ImageInspect - Add #nosec G118 for intentional context.Background() in stop goroutine Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * docs: add Go doc comments to exported symbols for docstring coverage Add missing doc comments to Store.Create, Store.GetByName, Store.ListByExecution, FilesystemTmpStorage.Put, FilesystemTmpStorage.DeleteByPrefix, and DockerRunConnector.Execute to reach 80% docstring coverage on PR-modified files. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: move #nosec G118 annotation to goroutine line where gosec flags it Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: address CodeRabbit review round 2 Artifact system: - Use DB clock (NOW() - interval) in ListExpired instead of app clock - Return generated ID/CreatedAt from Create via RETURNING clause - Add DeleteByID for per-artifact reaper cleanup - Reaper now deletes per-artifact instead of per-execution batch - Add path traversal protection to FilesystemTmpStorage.Put - Use filepath.Rel for stronger containment check in DeleteByPrefix - Add CHECK (size >= 0) to migration, remove redundant index - Fix missing error checks in tests - Fail in CI instead of skip when Postgres container fails Docker connector: - Wire TLS credentials (ca_cert, client_cert, client_key) into client - Wire registry_credential into pullImage for private registry auth - Check io.Copy drain error after ImagePull - Reject negative memory values in parseMemoryString - Validate mount source/target are non-empty - Log stdin pipe errors - Check errCh after status in ContainerWait select Engine: - Refresh artifacts in CEL context after each step completes - Clean up orphaned blob when ArtifactStore.Create fails - Emit audit event (artifact.persisted) after artifact storage Examples: - Fix postgres output field reference (output.json → output.rows) - Note continue_on_error limitation in volume backup (see #29) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: address CodeRabbit review round 3 - Add TmpStorage.Delete for single-file removal by URL - Clean up partial file in Put on io.Copy failure - Reaper uses Delete(url) instead of DeleteByPrefix - Stop mutating caller's params map in docker/run Execute - Use strconv.ParseInt for stricter memory value parsing - Reject artifact bind mounts for remote Docker daemons - Full rollback of persisted artifacts on partial failure in engine Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: address CodeRabbit review round 4 - Make TmpStorage.Delete idempotent (treat ErrNotExist as success) - Fail fast when step declares artifacts but tmp storage not configured - Use cli.DaemonHost() for remote daemon detection (handles DOCKER_HOST env) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: address CodeRabbit review round 5 - Reject symlinks and non-regular files in TmpStorage.Put - Validate cpus param is non-negative in docker/run - Add timeout context and error logging to container cleanup/stop - Create fresh artifacts scratch dir per retry attempt Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
This was referenced Apr 6, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
.map(),.filter(),.exists(),.all(),.exists_one()) that were never documentedCloses #14
Custom Functions
toLower(),toUpper(),trim(),replace(old, new),split(delim)parseInt(s),parseFloat(s),toString(v)obj(k1, v1, k2, v2, ...)flatten(list)default(value, fallback)— handles null, error, and unknownjsonEncode(value),jsonDecode(string)parseTimestamp(string)(6 formats),formatTimestamp(ts, layout)Key design decisions
cel.Libinterface ininternal/cel/functions.goparseTimestampnamed to avoid collision with CEL's built-intimestamp()parseTimestamptries 6 date formats: RFC3339, RFC3339Nano, bare ISO datetime, date-only, US date, named monthobj()uses fixed-arity overloads (2/4/6/8/10 args) since cel-go doesn't support true variadicdefault()handles null, error, and unknown valuesDocumentation
concepts/expressions.mdwith all functions and macrosgetting-started/data-transformations.mdwith 3 patterns (CEL-only, AI-powered, hybrid)data-transform-api-to-db.yaml,ai-data-enrichment.yamlTest plan
go vet ./internal/cel/clean🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
toLower,toUpper,trim,replace,split), type coercion (parseInt,parseFloat,toString), collections (default,flatten,obj), JSON (jsonEncode,jsonDecode), and timestamps (parseTimestamp,formatTimestamp)map,filter,exists,all,exists_oneDocumentation