# Taher WBA — Database Architecture Assessment

**Date:** 2026-05-21
**Source:** `web-entry-prod-2026-4-3.bacpac` (1.37 GB compressed) restored to SQL Server 2022, paired with the `WebEntry` repository.
**Scope:** SQL Server schema, indexes, types, audit infrastructure, sprocs/TVFs/views, and the EF Core 8 layer that owns them. UI and integration code are out of scope except where they ride on the data model.

This is a candid technical review, not a remediation plan. The goal is to give an accurate read on the *quality bar* of the data layer as it stands today.

---

## TL;DR

The schema is functional and the application clearly works in production, but the database is carrying a large amount of accumulated tech debt that is now actively costing the company money in storage, query performance, and — most importantly — **correctness risk on financial data**. The dominant issues are:

1. **`FLOAT` for money is used everywhere**, including columns that drive the general ledger sync to Sage. This is the single most serious problem in the schema.
2. **The audit table is broken and runaway.** 109 million rows; a bug in `TaherContext.OnSave.cs` means the intended skip-list for noisy tables never fires, so the `SiteInfo` heartbeat alone has produced ~8 million rows of audit churn.
3. **Loose typing throughout** — `nvarchar(max)` on short fields, no `datetimeoffset` on time-of-day data, denormalized strings baked into report tables.
4. **Index hygiene is inconsistent.** Some hot tables have 3–4 overlapping indexes totaling 4× the table size; others are heaps. At least one index is still named `missing_index_2755_2754`.
5. **Logic is split between EF Core and a sizable collection of hand-maintained sprocs/TVFs** with their own ad-hoc versioning scheme.

None of these are blockers to operations today. All of them will get more expensive to fix the longer they wait, and several are correctness risks rather than perf risks.

A grade is necessarily reductive, but for calibration: **C+**. The structure makes sense for the domain, the migrations history is clean and forward-only, and there's a real attempt at multi-tenant filtering, audit, and effective-dating. But the type system is undisciplined, the audit subsystem is buggy in a way no one has noticed, and there is no evidence that anyone is responsible for the database as an architectural artifact distinct from the C# model.

---

## 1. Domain & Shape

The system is a **K-12 / B&I food-service management platform** (Taher Inc.). The model centers on:

- **Org hierarchy**: `Region` → `District` → `Units` (operating contracts / sites) → `Buildings` → `CashRegisters`
- **Weekly accounting periods**: the `Period_*` family (`Period_SalesItem`, `Period_CostItem`, `Period_Labor`, `Period_General_Labor`, `Period_AdpLabor`, `PeriodRecords`, `PeriodCredit`)
- **Budgeting**: `BudgetPlans` + a small constellation of `BudgetPlan_*` tables, effective-dated and fiscal-year-aligned (July 1 start)
- **POS ingest**: `DailyCashRegisterInfo`, `CashRegisterReadings`, plus a Clover integration in `CloverUnits` / `CashRegisterCloverDevices`
- **Invoicing → GL sync**: `Invoices`, `InvoiceLineItems`, `InvoiceBatches`, `_GLAccount`, `_GLPeriodPostingHistories` (the bridge to Sage 100)
- **Payroll**: `Period_AdpLabor` (ADP integration)
- **Operational reporting**: `OpsRepSales`, `OpsRepCosts`, plus year-suffixed snapshots (`*2425`, `*2526`)
- **Support**: `ServiceTickets`, `ServiceTicketAttachments` (paired with Azure DevOps)
- **Audit**: a single `_AuditEntries` table for the whole system

The shape of the model fits the business reasonably well. The boundaries between operating data, planning data, and reporting data are visible. The instinct to keep effective-dated history (employee rates, building enrollments, budget plans) is right. The piece that's missing is *discipline at the column level*.

---

## 2. Volume & Storage Reality

**Database size after restore:** ~73 GB allocated (60 GB data + 14 GB log).
**Actual table data:** ~22 GB. The data file is roughly 2.7× over-allocated — the file has grown but free space inside it hasn't been reclaimed. The 14 GB log is also large for what is fundamentally a small-business system.

**Row counts (top of the list):**

| Table                       | Rows         | Size    |
|-----------------------------|-------------:|--------:|
| `_AuditEntries`             | 109,341,717  |  9.3 GB |
| `Period_CostItem`           |  53,400,473  |  7.7 GB |
| `Period_SalesItem`          |  12,753,259  |  3.9 GB |
| `Period_Labor`              |   2,613,382  |  460 MB |
| `_GLPeriodPostingHistories` |   1,221,311  |  151 MB |
| `OpsSalesTax`               |   1,204,261  |   86 MB |
| `OpsRepCosts`               |     940,014  |   65 MB |
| `Period_General_Labor`      |     814,198  |  109 MB |
| `InvoiceLineItems`          |     551,577  |   52 MB |
| `Invoices`                  |     372,472  |   59 MB |

Two takeaways:

- The audit table is 13% of the database. Together with its single secondary index (5.15 GB) it is **~20% of total storage on its own**, and that's after subtracting it from the working set the auditing was supposed to protect.
- The `Period_*` tables are the operating heart of the system. They are not unreasonably sized for what they represent (millions of unit-day-item rows over years), but their indexes are.

---

## 3. The `_AuditEntries` Problem

This deserves its own section because the failure mode is not "audit is slow" — it's "audit doesn't do what you think it does, and you've been paying for it for 18 months."

### The skip-list bug

`TaherContext.OnSave.cs` declares:

```csharp
private Type[] IgnoredAuditEntryTypes = new[] {
    typeof(AuditEntry), typeof(SiteInfo), typeof(CacheRefreshQueue),
    typeof(_GLAccount), typeof(_GLPeriodPostingHistory), typeof(ReportCache)
};
```

…and then checks membership with:

```csharp
if (IgnoredAuditEntryTypes.Contains(entry.Metadata.GetType())) continue;
```

`entry.Metadata.GetType()` does not return the entity's CLR type. It returns the runtime type of EF Core's metadata object (`RuntimeEntityType` in EF 8). The skip-list **never matches anything**. Every table the comment says is exempt has in fact been fully audited for the entire lifetime of the system.

You can see the consequence in the audit table itself. By `ReferenceObjectType` (top of the list):

| Type                                  | Audit rows  |
|---------------------------------------|------------:|
| `Taher.Model.PeriodSalesItem`         |  15,574,171 |
| `Taher.Model.PeriodCostItem`          |   9,832,462 |
| **`EntityEntry`**                     |   4,972,979 |
| **`Taher.Data.Model.SiteInfo`**       |   2,958,931 |
| `Taher.Model.Invoice`                 |     843,352 |
| `Taher.Model.InvoiceLineItem`         |     743,504 |
| `Taher.Model.PeriodLabor`             |     691,837 |

Almost **8 million rows on `SiteInfo` alone** (`SiteInfo` is a *singleton* settings table updated by the 10-minute AD-sync timer and on every Sage sync ping). Inspection of those rows confirms: they're heartbeat fields — `ADSyncActive`, `LastADSync`, `NextADSync`, `LastSageSyncDataReceived`. The audit table is recording metronome ticks.

The 5 million rows labeled `EntityEntry` are also SiteInfo updates — they bled through with a different type name because of a separate quirk in how `entry.Metadata.Name` is being resolved when the bug above also fails.

### Other problems in the audit code

- **Inserts are never audited.** `if (entry.State == EntityState.Added) continue;`. A row's creation leaves no trace; only modifications do. For a financial system this is a real audit-trail gap, not just a missing-feature.
- **Composite-key entities are silently dropped** unless the type happens to be `CashRegisterReading` (a hardcoded special case). Whatever else has composite keys is just not audited at all.
- **`AddRange(auditEntries)` is called inside the foreach loop** for each entry processed. EF deduplicates by reference identity so it doesn't *insert* duplicates, but the call is wasteful and confusing.
- **Old/new values are JSON-blobbed into `nvarchar(max)`**. Querying "what changed on this PeriodSalesItem between dates X and Y" requires a full table scan with `LIKE` over JSON text. The secondary index on `(LogTime, ReferenceObjectId, ReferenceObjectType)` helps locate the row but not the answer inside it.
- **No retention policy.** The table will grow forever at ~50k–170k rows/day. The recent 14-day window confirms peaks of 170k on heavy weekdays.

### Net assessment

The audit subsystem looks like it was built quickly, never verified end-to-end, and never revisited. The intended behavior is clear from the code (skip noisy singletons, audit all domain edits). The actual behavior is the opposite (audit the singletons most, skip many real entities). No one has noticed in 18 months.

---

## 4. Type Discipline

### `FLOAT` for money (the headline problem)

The database uses `float` (IEEE-754 double-precision binary) for **every monetary and rate column** I can find. A non-exhaustive list:

- `BudgetPlan_Cost.Amount`, `BudgetPlan_Labor.Amount`, `BudgetPlan_Revenue.Amount`, `BudgetPlan_GeneralLabor.Dollars/Hours`
- All of `BudgetPlans`: `FoodCost`, `FoodCostPerMeal`, `ManagementFee`, `RentFee`, `UtilitiesFee`, `Depreciation`, `Amortization`, `HealthInsurance`, `HourlyFringe`, `SupervisoryFringe`, `CreditCardFee`, `CreditCardFeeRate`, `SalesTaxRate`, `LiquorTaxRate`, `ADPTimeAttendenceFee`, `GAndAFee`, `InsuranceFee`, `MealEquivalent`, `MilkCostPerMealAccrued`, `NumberOfEmployees` (sic), `SCA`, `SubsidyRate`, `VehicleLeaseFee`
- All monetary fields on `CashRegisterReadings` — `CashSales`, `CreditCard`, `Checks`, `OverShort`, `Bills`, `Change`, `CampusCard`, `MealPlan`, `BlockPlan`, `LiquorSales`, etc.
- `Period_SalesItem.ActualAmount`, `BudgetAmount`, `PlanAmount`, `ProgramRevenue`, `FixedPriceRevenue`, `MealEquivalent`, `MealPrice`, `MealRatio`, `FederalReimbursement`, `StateReimbursement`, `SalesTaxRate`, `MilkCostAccrued`, etc.
- `Period_CostItem.ActualAmount`, `BudgetAmount`, `PlanAmount`
- `Period_Labor.ActualAmount`, `BudgetAmount`, `PlanAmount`, `EmployeeHourlyRate`, `HourlyFringe`, `SupervisoryFringe`, `OvertimeHours`
- `Period_AdpLabor.ActualAmount`, `PlanAmount`, `PayRate`, `WageAmount`
- All of the `OpsRepSales*` and `OpsRepCosts*` snapshot tables — `ActualDollars`, `BudgetDollars`, `PlanDollars`, `GLDollars`, `ActualCount`, `MealPrice`, `BillingRate`, `MealRatio`, `FederalReimbursement`, `StateReimbursement`, `SalesTaxRate`, `EquivalentRate`, `PlanCount`
- `UnitPayrollRelatedConfig`: `CheckCharge`, `Fica`, `Futa`, `WcRate*`, `UcRate*`, `Sers*`
- `Building_Enrollment.Enrollment`, `PeriodCredit.Amount/ReportValue`, `PeriodRecords.ActualFoodCost/PlanFoodCost/TotalMeals/CarryOverBalance`

This is **not safe for money**. `0.10` is not exactly representable in binary; neither is `0.2`, `0.3`, `0.4`, `0.6`, `0.7`, `0.8`, `0.9`, or most decimal cents. Repeated summation amplifies the error. The first place this typically surfaces is reconciliation against an external system that *is* using decimal arithmetic — which is exactly what's happening here, because `_GLPeriodPostingHistories` is the bridge to Sage 100, and Sage works in `decimal`.

If there has been a phantom penny problem on monthly closes that gets attributed to "rounding" or "edge cases", this is the cause. If there hasn't been one yet, there will be — the larger the totals get, the worse it gets.

For the avoidance of doubt: `Period_CostItem` has 53 million rows of `float` cost data. `Period_SalesItem` has 12.7 million rows of `float` revenue data. Both feed the GL sync.

### `nvarchar(max)` on short, bounded fields

The schema contains roughly **80 `nvarchar(max)` columns**. Many of them are clearly bounded text:

- `Buildings.AddressLine1`, `AddressLine2`, `City`, `State`, `Zip`, `Name`
- `Units.Name`, `Units.AzureId`, `Units.APUser`, `Units.DepartmentCode`, `Units.InventoryOwner`
- `Vendors.Number`, `District.AzureId`, `Region.AzureId`, `Employees.AzureId`
- `InvoiceBatches.BatchName`, `InvoiceCustomerNumbers.CustomerID`, `CloverUnits.CloverMerchantId`, `CloverUnits.CloverMerchantApiToken`
- Every name/string field on `OpsRepSales*` and `OpsRepCosts*` — `UnitName`, `DistrictName`, `RegionName`, `Name`
- `ServiceTickets.Name`, `Email`, `PhoneNumber`, `ContactEmail`, `NavigationURL`, `Description`, `UnitName`

The most striking case is `AzureId`. Azure object IDs are GUIDs. They could be `uniqueidentifier`. They are instead `nvarchar(max)`.

The costs of `nvarchar(max)` on short fields:

- Cannot be used in an index key (and can only be `INCLUDE`-d up to a limit).
- Forces SQL Server to use off-row storage when values cross the limit, fragmenting reads.
- Loses any chance of the engine making cardinality estimates from declared length.
- Defeats anyone trying to design a schema diagram or generate types from it — every short field looks like potentially a paragraph.

This is the kind of decision that EF Core makes by default when nothing tells it otherwise. It's a sign that no one has reviewed migrations with column-typing in mind.

### Date/time

154 columns are `datetime2`. **Exactly one column** is `datetimeoffset` — the audit log's `LogTime`. Everything else is wall-clock-without-timezone.

Taher operates across multiple US states. Even within one timezone, DST transitions create ambiguous and skipped hours. For period-boundary calculations (the system has a July 1 fiscal-year start and weekly periods) and for POS reads that happen at midnight, the lack of timezone info means downstream code is forever guessing what the value means. The current convention seems to be "everything is local server time, hope nothing breaks", which works until the production server moves region, or until someone runs analytics across units in different timezones.

`date`-only columns would also be more honest than `datetime2` for things that are clearly dates (`EffectiveDate`, `RecordDate`, `BatchDate`). Right now you can store a time component but it's never meaningful, which forces every consumer to call `.Date` defensively.

---

## 5. Indexing & Physical Layout

### Heap tables

The following tables have **no clustered index**:

- `_Calendar` — referenced by virtually every period/budget calculation
- `OpsRepCosts`, `OpsRepCosts2425`, `OpsRepCosts2526`
- `OpsRepSales`, `OpsRepSales2425`, `OpsRepSales2526`
- `OpsSalesTax`
- `BudgetPlan_GeneralLabor_Fabric`

Heaps in SQL Server accumulate forwarding pointers on update and never reclaim space without a manual rebuild. For a calendar table this is just a missed optimization; for the `OpsRep*` snapshot tables, which are large and rebuilt frequently by the cache layer, it's a measurable cost.

### Index bloat on `Period_CostItem`

53.4 million rows. The table itself (clustered index) is 7.7 GB. There are **four indexes** on top:

| Index                     | Size   |
|---------------------------|-------:|
| `$idx_FlashReport_1`      | 9.2 GB |
| `UC_Period_CostItem`      | 6.1 GB |
| `ix-cops2`                | 6.0 GB |
| `PK_Period_CostItem` (CI) | 7.7 GB |

Total ~29 GB on a 7.7 GB table. The largest index is bigger than the table. Without seeing the column definitions on each it's hard to be certain about overlap, but the size profile is a strong signal that the indexes are redundant — at minimum, `$idx_FlashReport_1` and `ix-cops2` look like they're covering similar query shapes.

### Auto-generated names that nobody renamed

`Period_SalesItem` has an index literally named `missing_index_2755_2754`. That is the SSMS Missing Index advisor's auto-generated identifier; it gets used when someone right-clicks the green hint at the top of a query plan and accepts the recommendation. Nobody renamed it afterwards. It's 4.5 GB.

This is a small thing but it's a tell. Indexes are being added reactively and not curated.

### Foreign keys without supporting indexes

Only one: `Period_Labor.EmployeeId`. Period_Labor has 2.6M rows; joining `Period_Labor` → `Employees` will full-scan. Easy fix, but it's been like this since 2023 and no one has caught it.

---

## 6. The Year-Suffixed Snapshot Tables

`OpsRepCosts` + `OpsRepCosts2425` + `OpsRepCosts2526` — and the same for `OpsRepSales` — are a classic *schema-by-year* antipattern. The structure is identical across all variants (`UnitId`, `PeriodStart`, `PeriodEnd`, `UnitName`, `Name`, `...Dollars`, `...Count`). What changes between tables is the fiscal year of the contained data.

This is the wrong shape. A single `OpsReport` table with an indexed `FiscalYear` column does everything these split tables do, plus:

- Queries spanning year boundaries (year-over-year analytics) don't need `UNION ALL` glue.
- Adding fiscal year 2027 is `INSERT`, not `CREATE TABLE` plus deployment plus updating every consumer.
- A partition scheme (or just a non-clustered index on `FiscalYear`) gives you the same physical isolation benefits without the schema fork.

Also: these tables carry **denormalized strings** — `UnitName`, `DistrictName`, `RegionName` are stored as `nvarchar(max)` in each row. If a region is renamed, every historical row of that region's reports drifts out of sync with `Region.Description`. The intent is probably "preserve the name as it was at report time" — which is a legitimate use case for denormalized history — but the implementation makes it impossible to tell whether a stored name is "frozen on purpose" or "stale because no one updated it." A `SnapshotName` column with an explicit comment would communicate intent; what's here doesn't.

---

## 7. EF Core ↔ Stored Procedure Split

The data layer mixes two paradigms:

- **EF Core 8** owns the core model, migrations (159 of them, dated April 2023 → Feb 2026), and most queries via LINQ + a small `BaseSpecification<T>` pattern.
- **A hand-curated SQL artifact tree** in `Taher.Data/SQL/` contains 15 sprocs, 14 TVFs, 3 views, and 23 one-off scripts, plus a small home-grown versioning scheme (`sp/<name>/v1.sql`, `v2.sql`, …).

The sprocs/TVFs are invoked from C# via `Database.ExecuteSql` in `PlanService`, `DataEntryService`, `TaherContext.OnSave.InvalidateCachedReports.cs`, etc. — at least 13 call sites. The split is along functional lines: declining-balance and flash-report calculations live in SQL, validation and aggregation live in C#. There's no obvious bright line and the boundary has shifted over time.

Observations:

- The versioning scheme is admirable in intent (each sproc has a sequential history alongside the EF migration that activated it) but it's redundant with what `migrationBuilder.Sql(...)` already provides via embedded resources, and it makes diffing painful — you have to look at `v13.sql` vs `v14.sql` rather than `git log -p`.
- Some functions (`tvf_GetCostBudgetForPeriod_All`) have grown to non-trivial size with multiple table variables and a clustered index *inside* the table variable. That works, but it's an indicator that the function has accreted optimization over time without a refactor.
- The `OnSave.cs` file has commented-out calls to `sp_SyncBudgetInventory` and `sp_UpdateDecliningBalanceValue`. The intended cache-rebuild-on-save path was disabled at some point and never replaced. This is dead code that masquerades as documentation of a feature that doesn't run.
- `tvf_GetCostBudgetForPeriod_All` uses `FLOAT` for `BudgetAmount` in its return table definition. So even the layer that exists *partly to produce financial reports* propagates the float-money problem.

The split isn't inherently wrong — there are good reasons to push some kinds of set-based work into SQL — but it lacks ownership. There's no obvious policy on when work goes in EF vs. when it goes in a sproc, and the sproc layer has tech debt of its own.

---

## 8. Tenant Filtering & Soft Deletes

### Query filters cover only some entities

`TaherContext.Tenant.cs` installs EF query filters on **9 entity types** — `BusinessUnit`, `Building`, `Employee`, `BudgetPlan`, `PeriodLabor`, `PeriodSalesItem`, `PeriodCostItem`, `InventoryRecord`, `PeriodAdpLabor`. The remaining 80 tables have no row-level tenant filter. That includes `Invoices`, `InvoiceLineItems`, `Comments`, `GLCodes`, `ServiceTickets`, `_AuditEntries`, `CashRegisterReadings`, `BudgetPlan_*`, and the entire `OpsRep*` family.

This isn't necessarily a bug — for many tables, access goes through a parent entity that *is* filtered, so transitively a user can't see data they shouldn't. But the gap is wide enough that the right model to have in mind is: **tenant filtering in this system is enforced by C# service code, not by the database.** The DB is willing to hand any row to any caller.

### `IgnoreQueryFilters()` is sprinkled liberally

There are **40 call sites** that bypass query filters explicitly, including in places that don't obviously need it (`UnitService.cs` loading vendors, `DecliningBalanceSummary.razor` loading cost items). Most importantly, `DataService.FindAsync<T>` **defaults `ignoreQueryFilters: true`**, so the most natural single-entity-by-ID lookup pattern silently bypasses tenant isolation unless the caller opts back in. This is the wrong default.

### Soft deletes

Soft-delete (`IsDeleted` + a recovery convention) is consistently applied: ~46 indexes start with `(IsDeleted, Id)`. The volume of soft-deleted rows is reasonable for most tables (4–34% depending on table), so the cost isn't catastrophic. The bigger smell is that the `EntityConstraints` method in `OnSave.cs` flips `Deleted` state back to `Modified` and calls `.Delete()` to set the flag — so the actual delete path goes through *Modified* and runs through the rest of the audit machinery. It works, but it conflates "this user soft-deleted this row" with "this user modified this row." The audit history reflects the latter.

---

## 9. Migration History

Quality observation, not a critique: the migration history is **forward-only and clean** — 159 migrations from `20230426144226_InitialSchema` through `20260216111720_AddFixedMonthlyCostDateRange`, no manual rollbacks, no rewrites, no `Designer.cs` mismatches I could spot. Whoever owns this has been disciplined about keeping the migration chain intact.

The schema mutations themselves tell a story:

- 95 of 159 migrations mutate existing tables (add column / rename / drop column). That's a high ratio. The model is evolving steadily, which is fine, but it also means there is no point where someone has stopped and consolidated the type-discipline issues above. They were baked in at `20230426144226` and have propagated.
- 34 migrations contain `CreateTable` / `DropTable`. New tables are still being added (`PeriodAdpLabor`, `UnitPayrollRelatedConfig`, `VendorInvoiceRelationship`, `AddReimbursementSettings`, `FederalReimbursementRates`, `StateReimbursementRates` — all 2025/2026). These newer tables have the same `float`-for-money pattern as the older ones, so the issue isn't "we didn't know better in 2023" — it's "no one is reviewing migrations against a typing standard."
- 200 migrations create indexes. This is fine, except it doesn't include any of the work to *consolidate* indexes that have piled up. There's no `DropIndex` ratio worth noting.

---

## 10. Things That Are Done Well

To be fair, this isn't a one-sided review.

- **Migrations are clean and continuous.** That's not nothing.
- **`uniqueidentifier` primary keys with `NEWSEQUENTIALID()` defaults** — the right call for distributed-friendly inserts without the page-split pain of random GUIDs. Clustered indexes on these PKs are well-behaved.
- **`AsSplitQuery()`** is used in a few places where it matters (notably the home-page unit dashboard). Someone knew about the Cartesian explosion problem with multiple `Include` chains.
- **`(IsDeleted, Id)` indexes** consistently support the soft-delete query pattern.
- **Effective-dated history** on `EmployeeRate`, `BuildingEnrollment`, `BudgetPlan` is a real attempt at temporal data modeling without using SQL Server temporal tables. The maintenance logic in `OnSave.cs` (sliding `EffectiveDateEnd` based on the next sibling row) is correct in shape, even if a bit fragile.
- **The TVF + table-variable + inline clustered-index trick** in `tvf_GetCostBudgetForPeriod_All` shows someone has at least one engineer who knows their SQL Server query plans.
- **The SQL artifact directory** (`Taher.Data/SQL/`) and the `MigrationSQLFile` helper, while redundant with EF's own embedded-resource support, demonstrates an intent to keep stored procedure history alongside C# history. The instinct is right.
- **`docs/security-concerns.md` and `docs/testing-concerns.md`** exist and are honest reads. The team is aware that the SA password is hardcoded, that the Sage integration endpoint uses a forgeable HMAC, that there are no real tests. The presence of those documents matters.

---

## 11. What This Says About Process

A schema is a fossil record. What it preserves:

- **EF defaults dominate.** When the C# property is `string`, the column is `nvarchar(max)`. When it's `double`, the column is `float`. There's no evidence that someone is reviewing the generated migration before applying it and asking "is this the right SQL Server type for the job?"
- **There is no DBA function.** Index proliferation, heap tables, year-suffixed snapshots, unrenamed advisor-generated indexes, the runaway audit table — all of these are things a DBA would catch on a quarterly review. The absence of these caught issues over three years strongly suggests no such review happens.
- **Validation against external systems isn't testing the math.** The Sage GL sync runs on production data with `float` monetary fields and apparently reconciles well enough that no one has raised an alarm. That tells me either: (a) totals are still small enough that the precision drift is below the threshold anyone is looking for, (b) Sage's tolerance for "close enough" hides it, or (c) it's silently miscategorizing pennies and someone is making the trial balance line up by hand. None of these scale.
- **The audit feature is decorative.** It exists, it has a UI somewhere (`EntryAuditPage.razor` in the WebEntry pages), and it's plugged in everywhere — but the broken skip-list means no one has ever audited what it actually records and found it weird. If someone had tried to use the audit log to investigate a real issue and noticed it was 30% noise on a singleton table, they would have filed a bug.

The honest summary is that the team that built this is a competent feature team that hasn't had data-architecture as a discipline. They added what the business asked for. The data layer is the cumulative residue of that work, plus EF Core's defaults.

---

## 12. Risk Ranking

If I had to rank the issues in this assessment purely by *how much they cost the business*, in order:

1. **`FLOAT` for monetary columns.** Correctness risk on financial data. Increases linearly with revenue. Already touches the GL sync.
2. **The broken audit skip-list + missing insert audit + no retention.** Storage cost is the visible symptom; the real cost is "your audit log isn't trustworthy when you need it."
3. **Index bloat + redundant indexes on the `Period_*` tables.** Slows writes (every modification updates 3–4 indexes), wastes ~20 GB.
4. **No timezone awareness.** Latent, but it bites once you have units across timezones or move infrastructure.
5. **`nvarchar(max)` everywhere short fields should live.** Mostly a craftsmanship and ergonomics issue; minor perf impact.
6. **Year-suffixed `OpsRep*` tables.** Mostly an operational maintenance burden; queries that ought to be simple become awkward.
7. **Partial tenant filtering + `FindAsync` default of `ignoreQueryFilters: true`.** Real security risk if a vulnerability ever lets an attacker control entity-by-ID lookups, but defense-in-depth rather than an open door today.
8. **Mixed EF / sproc ownership without a policy.** Annoyance more than risk; slows future refactors.

---

## Closing

The system works. The business runs on it. None of the issues above are existential. But the data layer has accumulated enough specific, identifiable problems — and several of them touch money — that this is a fair moment to stop adding features for a beat and have someone with database-architecture as their primary skillset look at it. The cost of doing nothing is not zero, and it compounds.
