[mforce]

What and why

Reports, exports, the audit viewer, and response provenance now read through IInsightsModule. ReportQueries, ExportQueries, and AuditEventRepository move from Platform's Infrastructure.Repositories into the Insights-owned Infrastructure.Insights namespace. Existing routes, authorization, results, streaming, and report concurrency behavior stay intact.

One shared facade removes every direct audit repository injection. It aggregates the existing read ports, which remain internal composition dependencies so the concurrency decorator and provenance tests retain their behavior. A guard prevents endpoint use of those ports. Audit listing maps to a DTO after materialization; the append sink remains in Platform.

Six new R ledger cells expose reads previously hidden by Platform's free hub. Insights reads Farm currency, Flock Management lifecycle and bird movements, Egg Operations production and grading, Commerce sales and payments, General Inventory exports, and Finance expenses. Explicit row types make the edges visible without changing SQL. The matrix is regenerated and both moved audit bypass entries are re-reviewed and re-keyed.

Removing the audit mapper and response constructor changes the walk from 405 to 403 adapters on the original baseline. Non-empty rows remain 150 and Insights reaches remain 25, with no Loosenable rows. ExpenseEndpoints keeps #1010's IFinanceModule and ExpenseAuditEntityType; only its provenance dependency changes to IInsightsModule. The Finance contract and adapter-reach guards pass with the combined ledger.

How it was verified

Verification on the original #856 baseline at 05d7e7a7 and on this branch after merging main at b9f7f76b:

Project	Before	After
Domain	495	495
Application	544	593
AppHost	10	10
Integration	1,873	1,873
Total	2,922	2,971

dotnet build Cluckwork.sln passed with zero warnings and errors. The baseline used sg docker -c 'dotnet test Cluckwork.sln'; all four projects passed. The merged branch passed all four projects in one solution run with a local xUnit MaxParallelThreads=2 runsettings file, zero failures or skips. No test timeout, assertion or repository configuration changed. The increase in Application tests includes #1010, now merged into main. The ImagePin filter matched no Application tests and passed both Integration tests.

Raw SQL literals are byte-identical; normalizing the namespace, explicit generic types, and DTO mapping reproduces all three original query files exactly. The export snapshot retains its separate non-retrying repeatable-read context. All five audit order clauses retain Sequence. Existing backend behavioral assertions are unedited; three C# test files change imports only.

The read-only guard rejects EF writes, SQL DML, unit-of-work and audit-write calls, direct endpoint use of query ports, and entity or IQueryable contract results. Real-source mutations failed on DbContext.SaveChangesAsync and an IQueryable<AuditEventRead> leak, then were reverted. The existing seam guard also rejected the leak. Restoring one direct audit repository injection also failed the endpoint-port guard. The Round 1 concrete UnitOfWork.SaveChangesAsync mutation now fails the real-tree assertion; the guard requires external Cluckwork.* targets to be approved reads, including calls made through method references, constructors, accessors, conversions and operators. Causal fixtures remain for the unit-of-work, delegate and constructor cases. Round 2 mutations all fail: both delegate forms, the external constructor/getter/conversion/operator, and the helper using a Domain namespace. Local read helpers remain green. The operation walk explicitly permits the existing read ports, contract DTO construction and getters, Domain-assembly data getters, and the snapshot/context read members. Dynamic calls and SQL method references without an inspectable literal fail closed. All mutations were reverted.

The #1009 month-boundary fix from #1011 is now present through main. This branch has no diff under tools/simulation/ui.

CI on the merged head passed the build, all four backend test legs, web typecheck/tests/build, all three Playwright shards, CodeQL, dependency review and both language analyses. The arm64 image check remains red under #1006: Trivy found two HIGH findings in the base image's OpenSSL packages (CVE-2026-84782) and no CRITICAL findings. The amd64 leg was canceled by the matrix and image publishing was skipped.

Checklist

• Conventional commit title.
• New guards passed, then rejected real mutations, then passed after restoration.
• Slice [C] #514 slice 14: Insights read facade for reports, exports and audit reads #856 is on epic Modular-monolith architecture design + implementation plan (from PR #423) #514.
• No credentials or hosting-provider names added.

Closes #856

[mforce]

Review of record: Codex gpt-6-astra, round 1, at head e4881d85. Posted by the coordinator. The head has since moved to 52ff9b43, which only removes the duplicate Playwright edits (src/ and tests/ unchanged). The log files named below are local to the review host.

PR #1012 review

Reviewed head e4881d8564b829d905b65802c41ef76553db6eab against 05d7e7a7, using the supplied diff and detached checkout. Excluded tools/simulation/ui/**, #1006, and #1009 as instructed. Read the PR, #856's superseding audit comment, AGENTS.md, and the five requested decision records. No delegation or GitHub writes.

No product defect found. One confirmed defect in the new architectural guard needs correction.

Finding

P2 · CONFIRMED · Concrete Infrastructure write calls escape the read-only guard

Location: tests/Cluckwork.Application.Tests/Architecture/InsightsReadOnlyTests.cs:125

The guard rejects write calls declared in Application or EF namespaces but accepts the same write through an existing concrete Infrastructure implementation.

Failure scenario: A read method calls await new UnitOfWork(db).SaveChangesAsync(ct);. Any pending changes on its shared request context are flushed, violating the read-only requirement, while every new Insights guard remains green. UnitOfWork.SaveChangesAsync directly calls db.SaveChangesAsync at src/Cluckwork.Infrastructure/Persistence/UnitOfWork.cs:7. This is a guard defect, not a write present in the submitted product code.

Evidence: Temporarily inserted that exact call at the start of the query in AuditEventRepository.ListAsync, then ran:

dotnet test tests/Cluckwork.Application.Tests --no-build --nologo \
  --filter FullyQualifiedName~InsightsReadOnlyTests

All 17 tests passed, including ReadSession_UsesOnlyApprovedPersistenceOperations. The guard's own Roslyn compilation resolved the added call without errors. As a control, inserting await db.SaveChangesAsync(ct); at the same location produced 16 passes and one failure, explicitly naming Microsoft.EntityFrameworkCore.DbContext.SaveChangesAsync. Both mutations were reverted. Logs: surviving concrete call, rejected direct call.

The namespace checks at lines 125–132 omit Cluckwork.Infrastructure; the scanner does not inspect the called implementation's body. Classify concrete implementations of write contracts as writes, or explicitly constrain external Infrastructure calls, and retain this concrete-call mutation as coverage. Adding another method-name denylist would leave the same delegation gap.

Product and architecture checks

• Behavior parity: Independently reversed the namespace, explicit generic arguments, and audit DTO mapping changes; all three query files then matched their originals byte for byte. Report arithmetic, CSV headers, column order, row projections, and queries are unchanged. The audit DTO contains the same eight fields previously mapped by the endpoint. Routes, validation, statuses, and authorization are unchanged. Exports and audit retain AdminOnly. See parity evidence.
• Snapshot: ExportQueries.BeginConsistentReadAsync still creates a separate context with UseNpgsql and no retry strategy, opens RepeatableRead, and assigns it to activeDb. Every export switch arm reads activeDb. WriteZipAsync opens the snapshot before resolving datasets and holds it through enumeration. The scoped facade forwards snapshot and dataset calls to the same scoped IExportQueries instance. No moved export query escapes the snapshot or switches to the retrying request context.
• Tenancy and flock scope: The existing context filters are unchanged. Production grade totals still correlate through filtered DailyEntries; scoped flock and movement reads retain their predicates. Direct child-table exports retain their existing Admin-only restriction.
• Audit ordering: All five tie-breaks remain: list, created, creator, latest, and promoted. All three provenance SQL strings are byte-identical. Sequence remains absent from audit responses, provenance DTOs, and export columns.
• Concurrency: ReportConcurrencyLimitFilter remains attached to the report group and acquires the existing singleton limiter by tenant.AccountId. The facade resolves the registered IReportQueries, preserving the test decorator and cap. The concurrency integration tests passed.
• Slice completion: None of the six named endpoint files still injects IAuditEventRepository; a search of all endpoint files found zero occurrences. The remaining repository dependency in InsightsModule is the intended internal composition dependency. ExpenseEndpoints changes only the audit dependency and its import, preserving refactor(finance): put Finance behind an IFinanceModule contract #1010's Finance work.
• Adapter counts: The head scanner reports 403 adapters, 150 non-empty rows, no Loosenable rows. Temporarily restoring only the baseline AuditEndpoints.cs yields 405 adapters and the same 150 rows. Removing ToResponse and the AuditEventResponse primary constructor accounts for the two declarations. Both have no module reach. Both ledger versions contain 25 Insights reaches; replacing Insights-owned ports with another Insights-owned port does not change those reaches. Logs: baseline declarations, restored head.
• Ledger: All six new cells are real R edges. Reports read Farm currency; reports and exports read Flock Management, Egg Operations, Commerce, and Finance; exports read General Inventory. No moved code writes those owners. Explicit type arguments expose dependencies to the syntax walk without changing query binding. The module guard found no undeclared or stale edges.
• Bypass registry: Reviewed each of the three raw provenance queries. Created and promoted filter AccountId, EntityType, and entity IDs directly. Latest filters once in scoped, from which its self-joins derive. The two re-keyed registry entries identify the chunk method and its forwarding wrapper; their revised explanations match the queries. Tenant-bypass tests passed.
• Matrix and tables: The committed coupling matrix exactly matches independent regeneration by RealTree_CommittedMatrixMatchesRegeneration. This verifies generated content, rather than inferring how the author edited it. No entity or mapping moved; Insights still owns no tables and AuditEvents remains Platform-owned. Table ownership and FK guards passed.
• Contracts and style: The facade exposes DTOs and existing read contracts, with no EF or Infrastructure types. Seam checks passed. DI registers the scoped facade in Program.cs:78. Changed namespaces are file-scoped with outside usings; no new conflicting namespace segment or MediatR dependency was introduced. The solution build passed with zero warnings and errors.

Verification

• dotnet build Cluckwork.sln --nologo: passed, zero warnings/errors. Log
• Entire Application project: 561 passed. Includes the new guard, seam, ledger, adapter, table-owner, matrix, and tenant-bypass checks. Log
• Integration through sg docker -c: 109 passed, zero failures/skips. Selected AuditTests, AuditProvenanceTests, ExportTests, ReportsTests, ReportsConcurrencyLimitTests, ReportQueryBoundingTests, FlockScopeTests, TenantIsolationTests, and ListChronologyTests. Log
• After reverting all mutations: 19 Insights/adapter tests passed, and git status --porcelain was empty. Log

The full integration suite and browser tests were not rerun for this review.

Nits

None.

Verdict: Request changes for the confirmed P2 guard gap; no product defect found.

[mforce]

Addressed the confirmed Round 1 P2 in fe3e89ac4440918bd582e9ed944d0fea4b4d06c7.

Infrastructure invocations are now rejected unless their implementation is in the source trees this guard scans. Local helpers remain permitted because their bodies are checked. No method-name denylist or production change was added.

Mutation evidence:

• The new concrete UnitOfWork fixture failed against the original guard (17 passed, 1 failed), then passed against the fix (18 passed).
• Inserted the exact await new UnitOfWork(db).SaveChangesAsync(ct); at the start of AuditEventRepository.ListAsync: ReadSession_UsesOnlyApprovedPersistenceOperations went RED, naming Cluckwork.Infrastructure.Persistence.UnitOfWork.SaveChangesAsync (17 passed, 1 failed).
• Reverted the mutation. The complete solution passed: Domain 495, Application 562, AppHost 10, Integration 1,873; zero failures/skips. The solution build had zero warnings/errors.

Only InsightsReadOnlyTests.cs changes in this commit; production and Playwright files remain untouched.

[mforce]

Review of record: Codex gpt-6-astra, round 2, at head fe3e89ac. Posted by the coordinator. The log files named below are local to the review host.

PR #1012 round-two review

Reviewed only fix commit fe3e89ac4440918bd582e9ed944d0fea4b4d06c7, the supplied fix-only diff, and the worker's reply. No delegation or GitHub writes.

The original concrete UnitOfWork bypass is fixed. Two confirmed P2 guard gaps remain. No product defect found: this commit changes only the guard, and the submitted read implementations do not perform the demonstrated writes.

Findings

P2 · CONFIRMED · The invocation-only walk misses method references and implicit calls

Location: tests/Cluckwork.Application.Tests/Architecture/InsightsReadOnlyTests.cs:121

The new Infrastructure check applies only to explicit invocation expressions, allowing persistence through a method group, delegate, constructor, getter, operator, or conversion.

Failure scenario: Add these valid lines to AuditEventRepository.ListAsync before its query:

Func<CancellationToken, Task<int>> save = db.SaveChangesAsync;
await save(ct);

The method reference is never checked; the invocation resolves to System.Func.Invoke, which is unrestricted. All 18 Insights guard tests pass. await Task.Run(db.SaveChanges, ct); also passes all 18. These variants use existing production types and require no helper or new dependency.

Evidence: Delegate guard log, method-group guard log. A temporary integration probe staged an audit row, called the mutated ListAsync, disposed that scope without saving, and queried a fresh scope. The row existed in Postgres. The identical probe against clean source failed because the row did not exist. See delegate persistence and clean control.

The same missing-call problem applies to implicit execution. A temporary, compiled Infrastructure helper outside the scanned directory called db.SaveChanges() in each of its constructor, getter, implicit conversion, and operator bodies. Each corresponding use below passed the real-tree guard, and each independently persisted the staged row in Postgres:

_ = new InsightsWriteProbe(db, persist: true);
_ = new InsightsWriteProbe(db).Saved;
_ = (int)new InsightsWriteProbe(db);
_ = new InsightsWriteProbe(db) + 0;

These were four separate mutations, not one combined execution. Logs: constructor, getter, conversion, operator. In contrast, explicitly invoking the same helper's Save() method was rejected.

Inspect symbol-bearing operations beyond invocations, including method references and implicit calls, before allowing execution outside the scanned implementations. Adding more invocation namespace cases does not address these bypasses.

P2 · CONFIRMED · An external write helper in an unclassified namespace remains unrestricted

Location: tests/Cluckwork.Application.Tests/Architecture/InsightsReadOnlyTests.cs:126

The external-implementation restriction applies only to the Cluckwork.Infrastructure namespace prefix, allowing an ordinary call to an unscanned writer declared in another namespace.

Failure scenario: A helper compiled into the Infrastructure project, but declared under Cluckwork.Domain.Common, exposes:

public static int Save(AppDbContext db) => db.SaveChanges();

Calling Cluckwork.Domain.Common.InsightsDomainWriteProbe.Save(db) from ListAsync passes the real-tree guard. The helper is outside the scanned directory, its namespace matches none of the classified prefixes, and its body is never inspected. This setup does not add an EF dependency to the Domain assembly; it demonstrates that declared namespace alone is not a reliable execution boundary.

Evidence: The temporary helper compiled with zero warnings/errors. The real-tree guard passed, and the Postgres probe confirmed persistence. An equivalent explicitly invoked helper in Cluckwork.Infrastructure.Persistence was rejected. The helper sources and exact commands are preserved in the reproduction script.

Use an explicit policy for external calls, or inspect their implementations where available, rather than treating every unclassified namespace as read-only. This finding requires a newly introduced helper; finding 1 already works with the existing production types alone.

Fix verification and controls

Mutation or check	Observed result
Original new UnitOfWork(db).SaveChangesAsync(ct) in real ListAsync	Rejected; 17 passes, one guard failure naming UnitOfWork.SaveChangesAsync
Direct NpgsqlCommand.ExecuteNonQueryAsync	Rejected by operation classification
NpgsqlDataSource.Create / CreateCommand / execute	Rejected by operation classification
ADO.NET DbCommand.ExecuteNonQueryAsync	Rejected by operation classification
Direct writer inside a helper declared in an Insights file	Rejected; its DbContext.SaveChanges body is scanned
Direct writer inside a constructor declared in an Insights file	Rejected; its body is scanned
New local read helper declared in an Insights file	Passed
Unchanged legitimate Insights implementation	All 18 guard tests passed

The rejected cases failed on the named operations, not invalid C#. The passing cases satisfied the guard's own semantic-compilation check. Dapper is not a current dependency; no Dapper bypass is claimed and no package was installed. The System.Func and System.Threading.Tasks.Task bypasses above are available in the current build.

No new false positive was found on current read code or the local read-helper control. The scanned-tree exemption does catch direct writes in helper bodies; its remaining weakness is incomplete discovery of the operations those bodies can execute.

Commands used:

dotnet test tests/Cluckwork.Application.Tests --no-build --nologo \
  --filter FullyQualifiedName~InsightsReadOnlyTests

dotnet test tests/Cluckwork.Application.Tests --no-build --nologo \
  --filter FullyQualifiedName~ReadSession_UsesOnlyApprovedPersistenceOperations

sg docker -c 'dotnet test tests/Cluckwork.Api.IntegrationTests --nologo \
  --filter FullyQualifiedName~InsightsReviewPersistenceProbe'

The original-bypass result is in this log. All helper-based mutations, sources, and commands are in the reproduction script. Six separate Postgres mutation probes passed; the clean-source control failed at its persistence assertion as intended.

Restored checkout

Every mutation was reverted and every temporary source/test file removed. git status --porcelain is empty at fe3e89ac. The restored solution build passed with zero warnings/errors, and the entire Application suite passed: 562 tests. The full integration suite was not rerun for this guard-only review.

Nits

None.

Verdict: Request changes for the two confirmed P2 guard gaps; the original UnitOfWork bypass is fixed, and no product defect was found.

[mforce]

Addressed both Round 2 findings in 6ff3218, with one commit limited to InsightsReadOnlyTests.cs.

1. The guard now walks Roslyn IOperation trees once and checks method references, constructors, property accessors, conversions and operators through their method symbols. Both delegate mutations, the constructor, getter, implicit conversion and overloaded operator now fail the real-tree assertion with the corresponding target named. The delegate and external constructor fixtures failed under the previous guard and pass with this change. Unresolved dynamic calls and SQL method references without an inspectable literal also fail closed.
2. Unapproved external targets in every Cluckwork.* namespace are rejected. The reviewer's Cluckwork.Domain.Common.InsightsDomainWriteProbe.Save(db) mutation fails with that exact symbol named. Existing read ports, contract DTO constructors/getters, Domain-assembly data getters and the reviewed snapshot/context read members remain approved. Local write helpers still fail; the local read helper stays green.

All temporary helpers and real-source mutations were reverted. The solution build has zero warnings/errors. All four test projects pass: Domain 495, Application 567, AppHost 10, Integration 1,873, with zero failures/skips. The original measured baseline was 2,922 tests; the final total is 2,945. CI is running on this head.

The review loop was stopped deliberately by the owner after two consecutive rounds that found no product defect (rounds 1 and 2 found only guard gaps). No round 3 will be triggered.

[mforce]

Change map: what this PR changes, before and after

Posted by the coordinator so this PR's structural change can be read at a glance. Reports, export files, audit history and routes are unchanged; only who calls whom changes.

BEFORE                                         AFTER

ReportEndpoints ──► IReportQueries             ReportEndpoints ─┐
ExportEndpoints ──► IExportQueries             ExportEndpoints ─┤
AuditEndpoints  ──► IAuditEventRepository      AuditEndpoints  ─┤
ExpenseEndpoints ─┐                            ExpenseEndpoints ┤
DailyEntryEndpts ─┤                            DailyEntryEndpts ┼─► IInsightsModule ◄── new
EggGradeEndpoints ┼─► IAuditEventRepository    EggGradeEndpoints┤      │
FlockEndpoints   ─┤   (for "created by /       FlockEndpoints  ─┤      ▼
SaleEndpoints    ─┘    last changed by")       SaleEndpoints   ─┘   InsightsModule (forwards)
                                                                     ├─► ReportQueries
                                                                     ├─► ExportQueries
                                                                     └─► AuditEventRepository

Where the 3 readers live:
  Infrastructure.Repositories                  Infrastructure.Insights
  (shared "Platform" area, so their            (owned by Insights, so the ledger
   reads of other modules were invisible)       now shows what they read)

Audit list returned:
  AuditEvent (DB object), mapped in the        AuditEventRead (plain record,
  endpoint to AuditEventResponse               same 8 fields), mapped in the reader

Ledger change (reads that were always there, now declared):

                ┌──► Farm              (currency for reports)
                ├──► Flock Management  (flocks in reports, exports)
   Insights ────┼──► Egg Operations    (production, egg lots)
   (reads only) ├──► Commerce          (sales, payments)
                ├──► General Inventory (feed, water, stock)
                └──► Finance           (expenses)

New guard (tests only): InsightsReadOnlyTests fails CI if any Insights reader writes to the database.

After this PR and #1010 both merge: ExpenseEndpoints

ExpenseEndpoints
 ├─► IFinanceModule    (#1010)  ← expenses and categories
 ├─► IInsightsModule   (#1012)  ← "created by / last changed by"
 ├─► IFlockRepository           ← flock name   (future Flock slice, #852)
 └─► IAccountRepository         ← currency     (future Farm slice, #851)

The two PRs overlap in three files: ExpenseEndpoints.cs, module-ledger.json and the generated coupling-matrix.md. Whichever merges second rebases and regenerates the matrix.

[mforce]

Merged main at b9f7f76b after #1010 into this branch as merge commit 6092f533d83c3edd21450fea3cef79dddb41db79.

The three conflicts are resolved: ExpenseEndpoints keeps main's IFinanceModule flow and ExpenseAuditEntityType, with only its three provenance dependencies switched to IInsightsModule; the ledger keeps Finance's contract and adapter rows alongside the six Insights R edges and adapter rows; the coupling matrix was regenerated by its guard. Finance's contract and adapter reach guards pass.

The solution build has zero warnings or errors. All four test projects pass with no failures or skips: Domain 495, Application 593, AppHost 10, Integration 1,873 (total 2,971). The ImagePin guard also passes.