Conversation
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
7151eb2 to
47cb19d
Compare
This comment was marked as outdated.
This comment was marked as outdated.
47cb19d to
5ea19e3
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
There was a problem hiding this comment.
Pull request overview
Introduces explicit variable typing for null / empty mappings (Null) and unknown/unresolved values (Any) in the Zeebe variable resolution pipeline, with accompanying fixtures and tests to validate type inference and propagation.
Changes:
- Extend FEEL-based type inference to return
NullfornullandAnyfor unknown/unresolved values, including union-type merging across multiple origins. - Add/adjust tests and BPMN fixtures to cover literal outputs, path resolution, passthrough, unresolved variables, and mapping/merging behavior.
- Refactor test assertion helpers to support both exact equality (
variableEqual) and subset assertions (variableInclude).
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
lib/zeebe/util/feelUtility.js |
Implements Null/Any typing, unresolved-path handling, and union-type merging/backfilling during reference resolution. |
lib/base/VariableResolver.js |
Updates type/info list merging logic for merged variables. |
test/globals.js |
Refactors chai assertions and adds variableInclude. |
test/spec/zeebe/ZeebeVariableResolver.spec.js |
Updates agentic fixture expectations and adds type-resolution coverage. |
test/spec/zeebe/Mappings.spec.js |
Updates expectations for Null/Any and adds new merge/mapping propagation tests. |
test/fixtures/zeebe/type-resolution.bpmn |
New fixture covering literal, path, passthrough, and unresolved type resolution. |
test/fixtures/zeebe/mappings/merging.bpmn |
Extends merging fixture to cover Null/Any propagation scenarios. |
test/fixtures/zeebe/ad-hoc-sub-process.agentic.bpmn |
Updates agentic fixture (provider config + additional tool tasks), impacting variable origins/types. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
lib/zeebe/util/feelUtility.js
Outdated
| if (mapping.source.startsWith('=')) { | ||
| return mapping.source.substring(1); | ||
| } |
There was a problem hiding this comment.
@barinali cf my change https://github.com/bpmn-io/variable-resolver/pull/74/changes#diff-36d9bcfb588ee3a00382ee039fdd5b6fcf85f9ef67e7230a547281444a1be0b9R260
@Buckwich I have updated this piece in a way it returns with the FEEL expression actually so that in the user interface I can display the unresolved FEEL expression as in the screenshot.
Check out data.userPrompt.prompt as reference in the screenshot;
This comment was marked as outdated.
This comment was marked as outdated.
if we do parse with camunda dialect do we also need to support protectedNameBuiltins? or why do we need to parse with the dialect. In general i would prever this lib to not do any parsing and move those parts over to feel-analyzer
im not really a fan of and we should come up with a common way to detect feel expressions. i think most open PRs do it slightly different (eg #83 (comment)) |
We need it to parse multi-line strings, cf. 1bf7ad3.
Yes, we'd need that, but it can also be a bug we're aware of, today, until we 🔽
Yes. We're not there yet, though. |
Compared to the initial intention of this pull request, what we want to ship has been changed in a reducing manner. So, we currently need (compared to what we already have in the main branch);
In the current main branch, what we can visualize is as below;
We'd like to show these variables as follows with a single exception of
|
|
Thanks both!
|
2eb6bd4 to
a5fd87a
Compare
@barinali for this we need the |
Ensures we recognize the full syntax and Camunda strings don't parse as `Null`: * multi-line strings * backtick variable escapes
3f90a03 to
f53631c
Compare



Proposed Changes
This pull request aims to introduce types for various scenarios;
Nullnullliteral value asNullAnyScreenshots
With changes in this PR, we can display variables as can be seen in the screenshot:
Try it out
Try out via Camunda Modeler
variable-int-upbranch.Checklist
Ensure you provide everything we need to review your contribution:
Closes {LINK_TO_ISSUE}orRelated to {LINK_TO_ISSUE}@bpmn-io/srtoolPulled #77 out of the scope of this PR.