Repository navigation
Fix #5451 — refuse a MapFromAggregate VectorProjection registered Inline - #5468
Merged
Merged
Conversation
…line VectorProjection's class remarks have always said MapFromAggregate requires ProjectionLifecycle.Async, and nothing enforced it. ValidateConfiguration only checked the conjoined tenant_id column, so an Inline registration built without complaint. Measured on an unfixed build rather than reasoned about, since the issue was diagnosed from reading the code: the first save writes NO ROW at all -- live aggregation reads committed events and the stream has none yet, so the aggregate is null -- and the second save writes "a body", the state as of the FIRST event, missing the NoteTitled that triggered it. Correct-if-async is "a body" then "a title a body". The embedding and content_text therefore lag one change behind for the life of the stream, the content hash records that stale text as current, and nothing anywhere fails. The refusal is narrowed to a map declaring an aggregate. An event-mapped projection takes its text from the event it was handed, which Inline has in full, and Marten supports that on purpose -- the class remarks call it "correct, not merely tolerated". Fisher and Polecat refuse every non-Async registration of theirs; this follows their lookup (walk options.Projections.All, match IProjectionWrapper.InnerProjection by reference, read the lifecycle) without copying their broader rule. Live is deliberately not covered. JasperFx's ProjectionGraph.Add already rejects Live for any plain IProjection, and Marten's overload does not reach that guard, so a Live registration is accepted silently -- a gap in the registration path for every IProjection rather than anything specific to this projection. Noted in the test remarks; it wants its own issue. Closes #5451 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01481eiEZg1Bv6pu4DrgPRg3
This was referenced Sep 22, 2026
Closed
This was referenced Oct 1, 2026
Closed
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 #5451. The diagnosis in the issue was right; this adds the measurement it said it lacked, then the refusal.
What an Inline registration actually did
The issue was written from reading the code, so I ran it before fixing it. Registering
NoteAggregateVectorProjection(the existingMapFromAggregatetest projection) Inline and readingcontent_textafter each save:content_textStartStream(NoteWritten("a body"))a bodyAppend(NoteTitled("a title"))a bodya title a bodyWorse than "lags one change behind", strictly: the first save writes nothing, because live aggregation reads committed events and the stream has none yet, so the aggregate is null. Every later save then writes the state as of the previous event. The content hash records that stale text as current, and nothing fails.
The fix
ValidateConfigurationnow refuses a non-Async registration when the map declares an aggregate type, following the lookup Fisher and Polecat already use: walkoptions.Projections.All, matchIProjectionWrapper.InnerProjectionby reference, readsource.Lifecycle. It fires atDocumentStore.For(...)viaAssertValidity, so this is a build-time failure, as the issue suggested.Narrowed deliberately. Fisher and Polecat refuse every non-Async registration of their vector projections; Marten supports Inline for event-mapped projections on purpose — the class remarks call it "correct, not merely tolerated", since an event-mapped projection takes its text from the event it was handed and Inline has that in full. Only
MapFromAggregateis unsound there, so onlyMapFromAggregateis refused.Tests
a_map_from_aggregate_projection_registered_inline_is_refused— asserts the message names bothMapFromAggregateandProjectionLifecycle.Async, so it points at the fix rather than just failing.the_same_projection_registered_async_still_buildsan_event_mapped_projection_registered_inline_still_builds— the guard that matters most, since over-refusing would break a supported configuration.Marten.PgVector.Testson net9.0: 101 passed, 0 failed (98 before, plus these three). The existingmap_from_aggregate_tests, registered Async, still passes — so the refusal does not over-fire.One thing found on the way, not fixed here
ProjectionLifecycle.Liveis also accepted silently for this projection. JasperFx'sProjectionGraph.Addalready rejects Live for any plainIProjection("Live cannot be used for IProjection"), but the overload Marten'sopts.Projections.Add(projection, lifecycle)takes does not reach that guard. That is a gap in the registration path for everyIProjection, not something specific toMapFromAggregate, so I left it alone rather than widening this fix to hide it. Happy to file it — say the word.🤖 Generated with Claude Code
https://claude.ai/code/session_01481eiEZg1Bv6pu4DrgPRg3