fix(intent): a count roll-up recomputes when a child changes parents - #6837
Open
delchev wants to merge 1 commit into
Open
fix(intent): a count roll-up recomputes when a child changes parents#6837delchev wants to merge 1 commit into
delchev wants to merge 1 commit into
Conversation
A `rollups:` entry with the default `op: count` got a create and a delete handler but no `-updated` one - the generator assumed a count can only move when a child is created or destroyed. A child changes parents by an ordinary EDIT of its parent relation, so re-parenting recomputed neither count: the parent that received the child never counted it. Emit the update handler for every op. The recompute is the same query in all three cases and it reads the child rows back from the store, so it is idempotent and never op-specific - an edit that touched nothing the roll-up reads finds the value unchanged and writes nothing. The parent a child moved AWAY from is still stale until roll-ups also consume the `-rekeyed` event, which is filed separately; this alone makes the new parent correct. Covered by a `GlueRollupCountTest` unit test on the emitted descriptors and by IntentEngineIT, which now asserts the generated `LoanMemberRollupOnUpdate` binds the child's `-updated` topic and recomputes exactly like the create handler. Closes #6820 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 #6820.
What was wrong
GlueIntentGenerator.buildRollupsemitted the-updatedhandler only forop: sumandop: latest:opdefaults tocount, so the default case was the excluded one — and it is the case that needs the update handler most. A child changes parents by an ordinary edit of its parent relation (a loan reassigned to another member, a task dragged to another sprint): no child is created, none destroyed, so nothing recomputed. The parent that received the child never counted it, and because a roll-up is recompute-on-event the counter stayed wrong until some unrelated child of that parent happened to be created or deleted — an off-by-one counter, which reads as plausible.The fix
Emit the update handler for every
op. Nothing else changes: the recompute is the same query in all three cases and reads the child rows back from the store (findAll(Criteria.create().eq("<Fk>", entity.<Fk>))), so the handler is idempotent — an edit that touched nothing the roll-up reads finds the value unchanged and, thanks to the existingchangedguard, writes nothing and publishes no parent update. The event set is a property of the construct, not of the aggregation, so the emission is no longer op-specific.The parent the child moved away from is still stale: the update event carries the row's current FK only. That needs roll-ups to consume the
-rekeyedevent (the mechanismaggregatesalready uses) and is #6819; this fix alone makes the new parent correct.Tests
GlueRollupCountTest(new, engine-intent) — a count roll-up emits exactly three descriptors, binding""/-updated/-deleted, all carrying the sameopand recompute criteria.IntentEngineIT.rollup_generates_create_and_delete_listeners_that_recompute_the_parent_count— now also asserts the generatedLoanMemberRollupOnUpdatebindsintent-test-Loan-Loan-updatedand recomputes exactly like the create handler.Verified locally:
IntentEngineIT50/50,IntentEmissionCoverageIT+ModelGenerationITgreen, engine-intent + ide-template unit tests 690/690,formatter:validateclean,-P releasejavadoc clean.Docs
components/engine/engine-intent/CLAUDE.mdand the intent assistant guide (in this PR) — three handlers, and the remaining-rekeyedgap.GlueGenerator.rollups("a count roll-up contributes no update entry, so the update handler ends up sum-only") now explains why the event is part of the coalescing key.🤖 Generated with Claude Code