Skip to content

docs: clarify Conditional Flows aggregations are JMESPath, not functions#983

Merged
MiroCillik merged 1 commit into
mainfrom
miro-AJDA-924-aggregations-jmespath
Jun 25, 2026
Merged

docs: clarify Conditional Flows aggregations are JMESPath, not functions#983
MiroCillik merged 1 commit into
mainfrom
miro-AJDA-924-aggregations-jmespath

Conversation

@MiroCillik

Copy link
Copy Markdown
Member

Jira issue(s): PROOF-XXX

The "one-click aggregations" framing in the Conditional Flows docs implied Sum / Minimum / Maximum / Average are dedicated function constructs. They are not — the function enum only accepts COUNT and DATE. Each aggregation is generated by the UI picker as a JMESPath expression in the task value field. Anyone authoring a flow via the API or as a template needs to know the JMESPath equivalents.

Changes:

  • Added a warning callout in the Dynamic Value section stating aggregations are not functions, with the explicit equivalents: Sum → sum(...), Minimum → min(...), Maximum → max(...), Average → avg(...).
  • Replaced the misleading "JSON equivalent" example so Sum of importedRowsCount is shown as a JMESPath aggregation in the task value (a type: task source), not a function block.
  • Reframed the COUNT example to make clear that COUNT and DATE are the only true functions (using a function block with operands), distinct from the JMESPath aggregations.

Note: not built locally — Docker daemon was unavailable. Change is plain Markdown plus the existing warning.html include; the branch CI build will confirm it compiles.

@linear

linear Bot commented Jun 22, 2026

Copy link
Copy Markdown

AJDA-924

@vercel

vercel Bot commented Jun 22, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
connection-docs Ready Ready Preview, Comment Jun 24, 2026 9:28am

Request Review

@MiroCillik MiroCillik requested a review from ondrajodas June 22, 2026 14:37

@ondrajodas ondrajodas 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.

Code review — clarify aggregations are JMESPath (AJDA-924)

The core of the change is correct — I verified it against the live keboola.flow schema via get_flow_schema: the function enum is exactly ["COUNT", "DATE"] (no SUM/MIN/MAX/AVG), so aggregations genuinely must be JMESPath in value, and avg is the correct JMESPath spelling. The JSON examples match the schema's functionCondition/taskCondition shapes, the Liquid warning.html include is markdownified so it renders fine, and the #date--time-function anchor resolves correctly (Kramdown drops the &). Build will not break.

Two Important wording/accuracy points and a few Minor style notes inline. Verdict: approve with minor fixes — none are blocking.

Comment thread flows/index.md Outdated
Comment thread src/content/docs/flows/index.md
Comment thread flows/index.md Outdated
Comment thread flows/index.md Outdated
Comment thread flows/index.md Outdated
…ot functions

Document that the Sum/Minimum/Maximum/Average picker aggregations are
JMESPath expressions placed in the task value field, not function blocks
(the function enum only accepts COUNT and DATE). Add the JMESPath forms,
a JSON example for the sum aggregation, and note the result.* (picker
tree) vs job.result.* (generated value) path prefix difference.

Re-authored onto the Astro/Starlight structure using a :::caution aside.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@MiroCillik

Copy link
Copy Markdown
Member Author

Note for reviewers — branch re-authored onto the Astro/Starlight migration

While addressing the review feedback I found that main has migrated Jekyll → Astro/Starlight: _config.yml and _includes/warning.html are gone, the page now lives at src/content/docs/flows/index.md, and warnings use Starlight :::caution asides instead of {% include warning.html %}. This branch was based on pre-migration main, so a plain rebase would have produced broken output (an orphan root flows/index.md, a dead _config.yml exclude).

I reset the branch to current main and re-applied the change onto src/content/docs/flows/index.md. The PR is now a single-file diff (+24/-1) and mergeable. Because the original commit hashes/line anchors no longer exist, I couldn't reply inline on each thread — here is how every point from @ondrajodas was carried over:

  1. Unverified picker-codegen claim (L147) — Softened to "behind the scenes the aggregation is stored as a source definition … can be expressed as a JMESPath aggregation", so it no longer asserts the exact emitted string.
  2. result.* vs job.result.* prefix (L156) — Added: "the picker tree displays paths rooted at result.* (for example result.output.tables), while the generated value expression is rooted at job.result.*."
  3. Emphasis density (L140) — Reduced to the bold heading plus one statement; the third emphasis is now "rather than a function block".
  4. &bull;/<br> one-off (L141) — Now a native Markdown list inside the :::caution aside (Starlight renders Markdown natively). Verified via astro build that it renders as a proper <ul>/<li>.
  5. "the only true functions" wording (L161) — Now "the only functions exposed via the function block".

Verified with a local Astro production build (npm run build): the aside renders, all four aggregation list items render as <li>, and the sum(...) JSON example is present.

@MiroCillik MiroCillik requested a review from ondrajodas June 24, 2026 09:46

@ondrajodas ondrajodas 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.

Approving — all earlier review points are addressed.

  • #1 picker codegen claim softened to "can be expressed as" ✔
  • #2 added the result.* (picker tree) vs job.result.* (generated value) prefix note ✔
  • #3 native Markdown list items instead of &bull;/<br>
  • #4 emphasis tightened ✔
  • #5 "the only functions exposed via the function block" ✔

Core claim stays verified against the live keboola.flow schema (function enum = COUNT/DATE only; aggregations are JMESPath in value). Clean re-author onto the Astro/Starlight :::caution aside. LGTM.

@MiroCillik MiroCillik merged commit a7632ea into main Jun 25, 2026
3 checks passed
@MiroCillik MiroCillik deleted the miro-AJDA-924-aggregations-jmespath branch June 25, 2026 08:32
@MiroCillik

Copy link
Copy Markdown
Member Author

Ja nevim k cemu tady vlastne jsme :D

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