fix(#435): ProjectionProgressRow.AgentStatus is nullable — no store persists it#519
Merged
Merged
Conversation
…ersists it AgentStatus was typed non-nullable string, but no event store can populate it today. Both Marten and Polecat create an agent_status column on the progression table and read it back, yet neither has a code path that writes one: - Marten ships mt_mark_event_progression_extended(), whose SQL does write agent_status/pause_reason/running_on_node — but the only reference to it is EventGraph.FeatureSchema.cs, which registers it as a SystemFunction so the DDL is created. Nothing invokes it. - Polecat has MarkExtendedProjectionProgress, which likewise writes the columns and is constructed only from tests (polecat#323). So agent_status reads NULL in both stores, and an implementer of ReadProjectionProgressAsync would have to invent a value to satisfy the type. Make it string? so a store with nothing to report can say so — symmetric with LastHeartbeat, already nullable and documented as "null when the store does not track a heartbeat for it". Caught before release: #518/#517 are merged but ProjectionProgressRow has not shipped (latest JasperFx.Events is 2.27.0), so this costs nothing now. Co-Authored-By: Claude Opus 4.8 (1M context) <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.
Answers the open question raised on polecat#324: "I could not verify that
AgentStatusandLastHeartbeatare actually available frompc_event_progression... please say so on #435 rather than stubbing quietly."Saying so. 🙂
Finding:
AgentStatushas no data source in either storeProjectionProgressRow.AgentStatusis typed non-nullablestring, but no event store can populate it today. Both stores model the column and read it; neither writes it.Marten ships
mt_mark_event_progression_extended(), and its SQL genuinely does write the columns:But the only reference to that function in the codebase is
EventGraph.FeatureSchema.cs:101, which yields aSystemFunctionso the DDL gets created. Nothing ever invokes it. MeanwhileProjectionProgressStatementselectsagent_statusandShardStateSelectorreads it intoShardState.AgentStatus.Polecat is the same shape:
MarkExtendedProjectionProgresswrites the columns and is constructed only from tests (polecat#323);PolecatDatabase.AllProjectionProgressreads them back.Net:
agent_statusis always NULL in both stores. An implementer ofReadProjectionProgressAsyncwould have to invent a value —"Unknown",""— purely to satisfy the type. That's the quiet stub polecat#324 asked us to avoid.(
LastHeartbeatis fine. Polecat does writeheartbeatin production. It's already nullable, and both stores only model the column under extended progression tracking, sonullis the honest answer when it's off.)Change
string AgentStatus→string? AgentStatus, plus docs explaining when it's null and a test pinning the semantics. Symmetric withLastHeartbeat, already documented as "null when the store does not track a heartbeat for it".Why nullable rather than trimming the field
polecat#324 floated trimming. Nullable seems better:
LastHeartbeatalready established "nullable = the store doesn't track this" for this exact record.nullreads as "no agent state persisted for this cell," which is precisely true.Happy to trim instead if you'd rather the record carry only fields with a live data source — it's a small change either way, and the reason to decide now is that this hasn't shipped yet: #518/#517 are merged but the latest released
JasperFx.Eventsis 2.27.0, which has noProjectionProgressRow. Cheap now, breaking later.Worth a separate look
That neither store writes agent state at all may itself be the bug — the columns, Marten's
_extendedfunction, and Polecat'sMarkExtendedProjectionProgressall exist and are all unreachable, which reads like a feature that was built and never connected. This PR only makes the contract honest about today's reality; wiring agent state up (if wanted) is a daemon-level design question for both stores. Tracked on the Polecat side at polecat#323.Tests
Full
EventTestsgreen on net9 + net10: 512 passed, 0 failed each.🤖 Generated with Claude Code