133 lines
6.4 KiB
Markdown
133 lines
6.4 KiB
Markdown
# Resource-control review of fin-hub results
|
||
|
||
Date: 2026-08-11
|
||
Reviewer: `resource-control` / Codex
|
||
Reviewed repository: `fin-hub`
|
||
Reviewed commits: `d2b9bc4^..2895077`
|
||
Primary workplans: `FIN-WP-0002`, `FIN-WP-0004`, `RESOURCE-WP-0003`
|
||
State: changes requested before accepting the v0.1 exchange contract
|
||
|
||
## Conclusion
|
||
|
||
The fin-hub result is directionally strong, but the v0.1 resource-cost
|
||
exchange contract is not yet ready for approval. Its authority matrix correctly
|
||
separates booked financial facts in fin-hub from resource identity, technical
|
||
usage, allocation evidence, forecasts, and optimization in resource-control.
|
||
The client-attribution and engagement-margin work is a useful foundation.
|
||
|
||
Accounting-integrity and schema gaps remain. In particular, existing reports
|
||
can combine incompatible currencies and environments, changed CSV imports can
|
||
duplicate booked cost, and the SQLAlchemy and SQLite engagement-price paths do
|
||
not enforce the same invariants. The exchange envelopes also need exact money,
|
||
correction, and record-type semantics before implementation.
|
||
|
||
Verification on 2026-08-10:
|
||
|
||
- fin-hub worktree was clean at review time;
|
||
- `uv run pytest -q` passed: 33 tests, one dependency deprecation warning;
|
||
- a diagnostic reproduced EUR and USD service rows being combined into one
|
||
EUR-labelled total.
|
||
|
||
## Findings and implementation pointers
|
||
|
||
| Severity | Finding | Required task |
|
||
| --- | --- | --- |
|
||
| High | Service reporting groups by `service_id` and month without preserving currency and environment dimensions. | `FIN-WP-0004-T07` |
|
||
| High | CSV import identity is based on path and mtime; a changed or forced import appends the same booked rows again. | `FIN-WP-0004-T08` |
|
||
| Medium | SQLAlchemy and SQLite engagement-price paths enforce different attribution, currency, amount, period, and revision invariants. | `FIN-WP-0004-T09` |
|
||
| Medium | Booked-cost envelope does not define net, tax, gross, credit, refund, correction, decimal precision, or rounding relationships. | `FIN-WP-0004-T02` |
|
||
| Medium | The common planning envelope's single `amount` is insufficient for infrastructure, labor, setup, usage, allocation, and savings evidence. | `FIN-WP-0004-T03` |
|
||
| Low | fin-hub's canonical `INTENT.md` does not yet express booked-cost authority, client reporting, or the resource-control boundary. | `FIN-WP-0004-T01` |
|
||
| Low | Workplan lifecycle language and frontmatter status disagree. | `FIN-WP-0004-T01` |
|
||
|
||
`FIN-WP-0004-T05` remains the acceptance gate: the first round trip must prove
|
||
that a cost is booked once, projected to resource-control, joined to technical
|
||
evidence, and returned as planning or variance evidence without becoming a
|
||
second financial fact.
|
||
|
||
## Detailed findings
|
||
|
||
### 1. Currency and environment mixing
|
||
|
||
`src/fin_hub/coupling/ops_hub.py` keys service buckets only by `service_id`
|
||
and monthly totals only by month. A diagnostic with a EUR production row and a
|
||
USD development row for the same service produced one EUR-labelled total.
|
||
|
||
Service aggregation must use at least:
|
||
|
||
```text
|
||
service_id × environment × currency
|
||
month × currency
|
||
```
|
||
|
||
Unattributed and attributed reports must expose the same dimensional rules and
|
||
reconciliation totals.
|
||
|
||
### 2. Duplicate booked cost on changed imports
|
||
|
||
`src/fin_hub/services/ledger.py` treats `(source_path, source_mtime)` as import
|
||
identity and inserts every row from every newly observed file version. This is
|
||
file-delivery deduplication, not financial-fact idempotency. A corrected file or
|
||
forced import can inflate burn, attributed cost, and margin.
|
||
|
||
Each booked row needs a stable `financial_fact_id`, source-document and
|
||
source-line identity, content fingerprint, and explicit append-only
|
||
correction/reversal relationship. Duplicate delivery must leave totals
|
||
unchanged.
|
||
|
||
### 3. Divergent engagement-price invariants
|
||
|
||
The SQLite command path validates attribution components, period, currency,
|
||
non-negative amount, source, and the current revision. The SQLAlchemy
|
||
`EngagementPrice` model does not enforce equivalent rules and can persist a key
|
||
that disagrees with its dimensions or an invalid price/revision state.
|
||
|
||
Both persistence paths should use the same domain validator and database
|
||
constraints, with concurrency-safe uniqueness for the current price basis.
|
||
|
||
### 4. Monetary contract semantics
|
||
|
||
The booked-cost schema must define whether credits are signed, whether gross
|
||
equals net plus tax, where discounts apply, how refunds/reversals work, how
|
||
unknown tax is represented, and which precision and rounding rule applies.
|
||
Exchange money should use decimal strings or integer minor units, not binary
|
||
floating-point numbers.
|
||
|
||
### 5. Record-specific planning schemas
|
||
|
||
The common envelope may stay small, but `forecast`, `allocation`, `usage`,
|
||
`optimization`, and `commitment_candidate` each need a concrete schema.
|
||
Infrastructure cost, internal labor, external services, setup cost, recurring
|
||
cost, technical units, allocation shares and residual, and expected savings
|
||
must not be overloaded into one ambiguous `amount`.
|
||
|
||
### 6. Canonical intent and lifecycle alignment
|
||
|
||
`INTENT.md` should reciprocate the boundary already recorded in
|
||
resource-control: fin-hub owns booked financial facts, budgets, commitments,
|
||
burn, runway, and viability signals; it consumes resource-control projections
|
||
without becoming authoritative for resource identity or technical allocation.
|
||
|
||
The workplan should no longer describe itself as proposed after its
|
||
frontmatter is active. Joint review remains incomplete until these review
|
||
results are incorporated and linked by both repositories.
|
||
|
||
## Decisions recommended for v0.1
|
||
|
||
- Keep client and infrastructure attribution identifiers in distinct,
|
||
versioned namespaces under one common envelope.
|
||
- Native service and workload identity stays with its owning repository.
|
||
Resource-control may issue an explicitly scoped portfolio surrogate only
|
||
where no native authority exists.
|
||
- Rename resource-control `actual` records to `usage_observation` or
|
||
`cost_observation`; reserve “booked actual” for fin-hub facts.
|
||
- Each producer owns its outbound schema. The shared contract pins compatible
|
||
versions and reconciliation rules; transport is selected after the backup
|
||
round trip validates the model.
|
||
|
||
## Approval condition
|
||
|
||
Approve the authority boundary in principle. Keep `FIN-WP-0004-T01` in
|
||
progress until the contract incorporates this assessment. Do not mark the
|
||
v0.1 exchange accepted until `FIN-WP-0004-T02`, `T03`, `T07`, `T08`, and `T09`
|
||
are complete and `T05` demonstrates an idempotent reconciled round trip.
|