134 lines
6.4 KiB
Markdown
134 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.
|