fix(intent): an unknown key is an error, never a silent drop (#6541) - #6748
Merged
Conversation
The typed mapping is Gson, which ignores unknown properties, so a key the model does not declare was accepted and discarded: parse 200, generate 200, code generation 201, publish 200, and the promise the author wrote simply absent at runtime. The seed-row half was worse - the CSV lost the column, and when that column was a NOT NULL FK the import skipped EVERY row, so a rates nomenclature imported as zero rows and downstream computations ran on empty data, with every pipeline step green. Two halves, one rule: - `UnknownKeyValidator` walks the RAW YAML tree against the model classes - declared fields, honouring `@SerializedName` - so the known-key set can never drift from what the parser actually maps. It follows typed structure only: a Map-valued property (a step's `args`, a `map:`/`defaults:` projection, a relation's `where:`, a widget's `at:`) carries names from the model being described, not from the intent schema, and stays opaque here. - `validateSeedRowKeys` matches a seed row's keys against the entity's own field and to-one relation names plus the `stage` marker - exactly what `CsvimIntentGenerator` emits columns for. Both messages name the key, where it sits and the nearest declared name, and say "(names are case-sensitive)" on a pure case slip - the case that opened the issue (`contributionScheme` for the relation `ContributionScheme`). The issues join the structural ones, so one parse still reports everything. Generalizes `rejectRemovedNumberKeys`, which special-cased three keys for exactly this reason; it stays, because its migration messages beat "unknown key". Verified red-first (9 of 12 unit cases fail without it) and against 63 real production intents plus every in-repo fixture - no false positives. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This was referenced Aug 15, 2026
Open
delchev
added a commit
that referenced
this pull request
Aug 16, 2026
…6751) Follow-up to #6541/#6748, which made an unknown key an error for every TYPED node and for seed rows. The maps it left opaque split in two, and only one half is genuinely author-keyed: - a `map:`/`defaults:` projection, a relation's `where:`, a widget's `at:` and a delegate's injected `fields:` carry names from the model being described - they stay opaque, forever; - a step's `args:`, a process `trigger:`/`abortOn:`, a glue `event:` binding (incl. the nested `onStepReached: { process, step }`), a posting's `rule:`, a generate child's `forEach:` and a lookup's `between:`/`found:`/`notFound:`/ `ambiguous:` are closed vocabularies that merely happen to be typed as `Map` - an invented key there was as silent as anywhere else. The BPMN was emitted, the task existed, and only the behaviour the author asked for was missing. The second group is now checked: the fixed vocabularies from the registry `UnknownKeyValidator.MAP_KEYS`, and a step's `args:` per KIND (`IntentParser.STEP_ARGS_BY_KIND`), because its vocabulary depends on the sibling `kind`. A step therefore gets two messages - `declares unknown arg [assigne] - did you mean [assignee]?` for a typo, and `declares arg [if] but is a userTask - if is a decision argument` for a misplacement, which is the same silent drop. The six keys whose misplacement already has a dedicated, better-worded validator (setField/setRelationField/delegate/notify/timeout/ expire) stay in the vocabulary but are excluded from the generic line, so nobody is told the same thing twice. Unlike #6748's reflected sets these lists are AUTHORED, which is why this was a separate change: a missing entry rejects an intent that works today. Every entry mirrors what the readers actually consult, and the notify block validates against `NotificationIntent.BLOCK_KEYS` - the set its own `fromMap` reads, pinned to the class's embeddable properties by a test, so a new block property cannot ship unauthorable. Verified red-first (10 of 13 unit cases fail without it) and against the corpus sweep: 259 in-repo fixtures + 63 real production intents carrying 389 authored `args:` blocks - no false positives. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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
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.
Closes #6541.
The failure
The typed mapping is Gson, which ignores unknown properties. A key the model does not declare was therefore accepted and discarded — parse 200, generate 200, code generation 201, publish 200, and the promise the author wrote simply absent at runtime.
The seed-row half (the case that opened the issue) is worse: the emitted CSV loses the column, and when that column is a NOT NULL FK, CSVIM then skips every row. Observed end-to-end cost: a rates nomenclature imported as zero rows, downstream computations running on empty data, every pipeline step green. The trigger was a pure case slip —
contributionScheme:for the relationContributionScheme.The fix
Two halves, one rule — an unknown key is a validation error naming the key, where it sits, and the nearest declared name.
UnknownKeyValidatorwalks the RAW YAML tree against the model classes — declared fields, honouring@SerializedName(extends) — so the known-key set can never drift from what the parser actually maps. It generalizesIntentParser.rejectRemovedNumberKeys, which special-cased three keys for exactly this reason (that check stays: its migration messages beat "unknown key").IntentParser.validateSeedRowKeysmatches a seed row's keys against the entity's own field and to-one relation names, plus thestagemarker — exactly whatCsvimIntentGeneratoremits columns for. A collection relation (no FK column) is rejected too.A pure case difference is called out explicitly (
(names are case-sensitive)), being the slip hardest to see by eye. The issues join the structural ones, so one parse still reports everything to fix:Scope, deliberately
The walk follows typed structure only. A
Map-valued property carries keys drawn from the model being described, not from the intent schema — amap:/defaults:projection, a relation'swhere:, a widget'sat:— and stays opaque here; those are the per-feature validators' vocabulary.One member of that family is a real remaining gap: a process step's
args:keys are kind-dispatched (assignee/form/if/then/else/timeout/expire/setField/delegate/ …) and are still silently dropped when misspelled. Closing it means an authored per-kind key set rather than a reflected one, which is a separate change with its own false-positive risk; filed as #6749.Verification
.intenttest resources) — zero unknown-key hits, and a deliberate canary was caught.mvn test -pl components/engine/engine-intent: 557 tests green.IntentEngineIT(HTTP, smoke set): 47 tests green, including the newparse_rejects_an_unknown_key_and_an_unknown_seed_row_keyasserting the exact 422 messages — the outermost layer this change is observable at.formatter:validateclean; javadoc clean under-P release.Docs
🤖 Generated with Claude Code