Skip to content

refactor(arch): move Insights into Modules/Insights - #1089

Merged
mforce merged 4 commits into
mainfrom
refactor/1087-insights-module
Oct 5, 2026
Merged

mforce merged 4 commits into
mainfrom
refactor/1087-insights-module

Conversation

@mforce

@mforce mforce commented Oct 5, 2026

Copy link
Copy Markdown
Owner

Refs #1087

S1 of the package-by-module design: Insights moves into Modules/Insights in Application, Infrastructure and Api. This is a pure move with no behaviour change. The S0 codemod (tools/module-move/move.py) produced it from a finite table, and every proof below compares this head with the base 29f6003e.

Review order

  1. ea9bab23 split (mechanical): the seven report records leave IReportQueries.cs, and ExportDataset leaves IExportQueries.cs, each into its own file in the same namespace. Each declaration is a verbatim slice of the original, and the remainder equals the original minus those slices.
  2. 03ae000a move (generated; skim it): 24 git mvs, namespaces taken from the folders, and references, usings, ledger strings and paths rewritten by the tool. The Insights rules file is the only hand-fed input (see below). The tool added no unused using.
  3. 64b7e5a2 fix (read this one): the two path-pinned guards (InsightsReadOnlyTests, ExportSnapshotSourceTests), two comments that name moved files (web/src/i18n/en.ts, reports-range.spec.ts), and three src/AGENTS.md sentences that say where a moved module's contract and registration live.
  4. 41a99ed6 marks: the 11 Insights [ModuleContract] marks deleted. Their types now get contract status from the Contracts/ folder.

No re-key commit: the tenant-bypass rows changed only Symbol/File strings, and the 40 allow-list hashes still verify.

Change map

Before After
Application/Features/{Audit,Export,Insights,Reports} Application/Modules/Insights/Contracts/ (11 contract types: IInsightsModule, AuditEventRead, EntityProvenance, ExportDataset, 7 report records)
Application/Modules/Insights/Reports/IReportQueries.cs, Export/IExportQueries.cs, Audit/IAuditEventRepository.cs (outside Contracts, as src/AGENTS.md requires)
Infrastructure/Insights/* (4) Infrastructure/Modules/Insights/Repositories/*
Api/Endpoints/{Audit,Export,Reports}/* (5) Api/Modules/Insights/{Audit,Export,Reports}/* (still Platform-owned adapters)
Api/Hosting/Modules/InsightsModuleServiceCollectionExtensions.cs Api/Modules/Insights/InsightsModuleServiceCollectionExtensions.cs
Insights rules Namespaces: 5 feature namespaces Cluckwork.Application.Modules.Insights, Cluckwork.Infrastructure.Modules.Insights

The six Insights edge cells keep their reasons. Their Symbols now name Cluckwork.Infrastructure.Modules.Insights.Repositories.*.

Proofs (base 29f6003e vs head 41a99ed6)

Proof Result
dotnet build Cluckwork.sln (analyzers on, warnings are errors) 0 warnings, 0 CW diagnostics
EF migrations has-pending-model-changes "No changes have been made to the model since the last migration." No migration added, no Migrations/ diff
tools/schema-docs/generate.sh --check "docs/schema/ is up to date" (no content change, as the design predicts: the docs hold no CLR names)
Ordered DI descriptors, Production/Development × serving/one-shot × Redis absent/configured (8) identical after the type map
Generated OpenAPI document (Development) identical
Exact normalized contract set per owner (Astra, #1088) identical: 160 types; Insights 11, now under Modules.Insights.Contracts
Module-edge report (live edges) identical after the map
Peer-reach report identical
Table-owner report identical
Adapter-reach report live reach, undeclared, loosenable, persistence and contract-bypass lists identical. One expected difference: WalkedAdapterCount 402 → 403, because InsightsModuleServiceCollectionExtensions.AddInsightsModule now sits under the adapter root Cluckwork.Api.Modules. Its reach is empty and the persistence ban passes
Tenant-bypass registry Symbol/File strings follow the move; no hash changed (no Insights query spells a namespace)
coupling-matrix.md unchanged (CouplingMatrixRealTreeTests green)

The type map covers every top-level declaration in the 24 moved files, read at the split commit. Positions (file, line) are excluded from the report comparison.

Test counts (measured at base and head)

Run base head
Application.Tests: Architecture + TenantBypass + Documentation + ImagePin (includes NamespaceFolderAgreementTests) 474 474
Integration, one class at a time: ServingGuardCoverageTests 7, TrackedMutationReadTests 2, SchemaDocsTests 4, ReportsConcurrencyLimitTests 1, ExportTests 11, AuditProvenanceTests 48, ReportQueryBoundingTests 2, FlockScopeTests 16, ListChronologyTests 1 same all green, same counts

The full integration suite and e2e-smoke run only in CI.

Mutations (scratch worktree at the head; each case reset, built and run)

# Mutation Result Contract-set diff vs base
M1a codemod moves Contracts/EntityProvenance.cs (file and namespace) back to Modules/Insights/Audit/ red AdapterReachRealTreeTests (endpoints reach a non-contract Insights type), CouplingMatrixRealTreeTests Insights −EntityProvenance
M1b same file moved to Audit/ but keeps the .Contracts namespace red NamespaceFolderAgreementTests only none: this is the gap the folder rule closes
M2a ReportEndpoints.Sales injects IReportQueries instead of IInsightsModule red AdapterReachRealTreeTests, InsightsReadOnlyTests.Endpoints_DoNotUseTheReadSessionPortsDirectly, CouplingMatrixRealTreeTests none
M2b ReportEndpoints.Sales also injects IAuditEventRepository same three red none

Notes for the next slices

  • A registration file moved under Api/Modules/<Owner>/ becomes an adapter and is subject to the persistence ban. None of today's registration files resolves AppDbContext in a factory. A slice whose registration does must handle that.
  • The estimate recalibration after S1 is for the coordinator. This slice ran without hand fixes beyond the two path-pinned guards and two comments.

mforce added 4 commits October 5, 2026 19:44
…iles

move.py split: the seven report records leave IReportQueries.cs and
ExportDataset leaves IExportQueries.cs, each into its own file in the same
namespace. Each declaration is a verbatim slice of the original.

Refs #1087
move.py move with the slice's table: Application, Infrastructure and Api
files go under Modules/Insights, contract types under Contracts, with the
namespace taken from the folder. References, usings, ledger strings and
paths are rewritten by the tool; the Insights rules file now names
Cluckwork.Application.Modules.Insights and
Cluckwork.Infrastructure.Modules.Insights. The tool added no unused using.

Refs #1087
…e moved files

InsightsReadOnlyTests and ExportSnapshotSourceTests read the Insights read
session by path. Two comments name moved files. src/AGENTS.md now says
where a moved module's contract and registration live.

Refs #1087
The eleven Insights contract types now sit in Modules/Insights/Contracts;
the folder makes them contract, and a type with both fails.

Refs #1087
@mforce mforce added this to the Modular monolith milestone Oct 5, 2026
@mforce

mforce commented Oct 5, 2026

Copy link
Copy Markdown
Owner Author

Approve this slice at 41a99ed6, reviewed against 29f6003e. No findings through P3. The behavior-preserving move matches the Insights design. Both thermo-nuclear-code-quality-review and ponytail-review are complete on the non-move changes.

The independent rename-aware comparison confirms all 24 moves. All eight extracted declarations are verbatim, including comments, and the original files retain exactly their remaining content apart from blank-line compression. Beyond namespace/import changes, the moved files contain only:

  • The 11 planned [ModuleContract("Insights")] deletions.
  • Three qualified read-port type-name updates in the unchanged DI registrations.
  • Import separator blank lines in IReportQueries.cs and IExportQueries.cs.

No other moved-file content changes. The eight declarations are ProductionDay, GradeTotal, ProductionReport, SalesSummary, ExpenseCategoryTotal, ExpenseSummary, ProfitReport and ExportDataset.

Independently reproduced proof Result
Solution build, base and head Zero warnings and errors
EF migrations has-pending-model-changes, base and head No pending changes
Ordered DI, Production / serving / Redis configured All 450 descriptors identical after the type map
Ordered DI, Development / one-shot / Redis absent All 459 descriptors identical after the type map
Exact normalized contract sets, every owner Identical, 160 total and 11 Insights
Development OpenAPI Byte-identical, 111,367 bytes
Module-edge, peer-reach and table-owner reports Identical after type and position normalization
Tenant-bypass registry Only Symbol/File strings changed; all 40 hashes unchanged

The adapter count increase from 402 to 403 is benign. AddInsightsModule now enters S0's existing Cluckwork.Api.Modules adapter root. It has empty reach and no forbidden persistence access. Live reach and enforcement results remain identical. One clarification to the PR's proof table: the new method also adds an informational unresolved IServiceCollection entry. The scanner does not treat unresolved framework type names as violations; this introduces no guard or behavior regression.

The two edited guards retain their assertions and floors and read the same moved implementations. InsightsReadOnlyTests still scans the entire Api project for forbidden read-port references. ExportSnapshotSourceTests retains its snapshot-field mutation checks. The three edited src/AGENTS.md sentences accurately describe registration locations, contract derivation and keeping the read ports outside Contracts. The six edge reasons, migrations, schema docs and coupling matrix are unchanged.

Reproduced mutation Result
Move EntityProvenance's file and namespace out of Contracts Adapter and coupling-matrix guards fail; exact Insights contract set loses only EntityProvenance
Move only its file, retaining the Contracts namespace Folder/namespace guard fails; contract set stays unchanged
Sales endpoint injects IReportQueries Adapter, Insights read-only and coupling-matrix guards fail; contract set stays unchanged

All mutations compiled before testing and were restored. The restored solution build and affected guards passed. In total, 145 existing targeted tests passed at head, with classes run individually, polling enabled, and integration tests through sg docker -c. I did not run the full integration suite, browser suite or schema generator.

No structural or overengineering finding meets the requested threshold. Ponytail-review: Lean already. Ship.

Local evidence is preserved in /home/mforce/.cluckwork-slices/reviews/1089-41a99ed6-astra-evidence/, including 01-move-and-quality.md, 02-verification.md, the independent dumps, comparison scripts and mutation logs. No implementation fix, commit or push was made.

@mforce
mforce merged commit d4175b5 into main Oct 5, 2026
37 of 46 checks passed
@mforce
mforce deleted the refactor/1087-insights-module branch October 5, 2026 20:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant