Bound the reconciliation query for audit records without a batch id - #576
Merged
cdcavell merged 1 commit intoSep 22, 2026
Merged
Conversation
ReconcileAsync OR-ed records without a mutation batch id into the bounded batch query, so every such record ever written was loaded on every pass of the hosted reconciliation loop. Those records are now read by a separate query, newest first, capped by the new MaximumMalformedRecordsPerRun option (default 1000, validated 1-10000). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
cdcavell
deleted the
fix/bound-malformed-audit-record-reconciliation-query
branch
September 22, 2026 00:05
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Resolve the two moderate findings in ApplicationAuditReconciler.cs.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
Open (1)
What changed in this PR
This PR bounds reconciliation of audit records without a batch ID to prevent unbounded per-run loading.
Changes:
- Splits batched and malformed-record queries, limiting malformed records newest-first.
- Adds and validates
MaximumMalformedRecordsPerRun(default 1000). - Adds regression coverage and changelog documentation.
| File | Reviewed changes |
|---|---|
tests/ProjectTemplate.Web.Tests/ApplicationAuditReconciliationTests.cs |
Tests newest-first bounded processing. |
src/ProjectTemplate.Infrastructure/Data/Extensions/ApplicationAuditReconciliationServiceExtensions.cs |
Validates the new option range. |
src/ProjectTemplate.Infrastructure/Data/Auditing/ApplicationAuditReconciliationContracts.cs |
Defines the new option and default. |
src/ProjectTemplate.Infrastructure/Data/Auditing/ApplicationAuditReconciler.cs |
Applies bounded malformed-record retrieval. Two moderate findings remain: align whitespace-only ID predicates (2 votes) and add a supporting top-N index or equivalent access path (1 vote). |
CHANGELOG.md |
Documents the behavior change. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| // otherwise every such record ever written would be loaded on every reconciliation pass. | ||
| List<AuditRecord> malformedRecords = await _dbContext.AuditRecords | ||
| .AsNoTracking() | ||
| .Where(record => record.MutationBatchId == string.Empty) |
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.
Summary
ApplicationAuditReconciler.ReconcileAsyncloaded audit records withbatchIds.Contains(id) || MutationBatchId == "".MaximumBatchesPerRuncapped only the first half of that filter, so every audit record ever written without a batch id was loaded into memory on each pass ofApplicationAuditReconciliationHostedService. That load grew without limit.Changes
batchIds. Records without a batch id now have their own query, ordered newest first (ModifiedOnUtc, thenId) and capped withTake(MaximumMalformedRecordsPerRun).ApplicationAuditReconciliationOptions.MaximumMalformedRecordsPerRun(default1000). It is validated at startup to the range 1–10000, the same asMaximumBatchesPerRun.Findings already raised for older malformed records stay in the table and stay open. They just stop being refreshed once they fall outside the per-run window. The resolve step only runs for batched records, so these findings are never auto-resolved.
Tests
ReconcileAsync_RecordsWithoutBatchId_AreBoundedPerRunNewestFirst. With a cap of 2 and 5 records, only the 2 newest get findings.dotnet build -c Release(warnings as errors) passes;dotnet test: 456/456 passed.🤖 Generated with Claude Code