[verified] Fix backtest accounting identity + honest UI disclosure

Correct the multi-rebalance backtest accounting so ending wealth is
capital + price_pnl + dividend_income with no double counting:
- price_pnl now measures equity change excluding dividends (was reusing
  ending holdings value as 'price profit')
- dividend proxy is included in final_value and net_return, exposed as
  dividend_method=final_holdings_yield_proxy
- regression tests: flat price => zero price_pnl; flat + dividend =>
  dividend-only return; rising no-dividend => correct bucket P&L;
  multi-rebalance accounting identity
- UI (result card + saved-run history) labels dividends as ประมาณการปันผล
  (Proxy) and shows descriptive non-PIT badge when leakage_guard=false

Backend 239 tests passed; targeted backtest 11 passed; frontend build,
npm audit (0), static scan and diff check passed; fresh independent
review deleg_10918fed passed with empty blocker arrays.
Backtest remains descriptive non-PIT (leakage_guard=false) with the
default current-score scorer.
This commit is contained in:
Kunthawat Greethong
2026-08-27 09:00:39 +07:00
parent b362cc35bf
commit 1b971ac72d
8 changed files with 337 additions and 7 deletions

View File

@@ -157,6 +157,42 @@ NEVER include API keys, tokens, passwords, secrets, credentials, or connection s
---
## Session 2026-08-27 — Backtest Accounting Remediation
**Branch:** `main` · **Base HEAD:** `b362cc3` · **Commit/push:** not performed
### Completed
- Added RED/GREEN flat-price accounting invariants in `backend/tests/test_backtest.py`.
- Corrected general backtest price P&L to compare ending pre-dividend equity with initial capital.
- Included the final-holdings dividend-yield proxy in `final_value` and `net_return`.
- Added machine-readable `dividend_method=final_holdings_yield_proxy`.
- Updated the Vue backtest panel to label estimated dividends and warn when `leakage_guard=false`.
### Verified
- RED: flat-price regressions failed with incorrect `price_pnl=20,000` and `49,000`.
- GREEN: flat/no-dividend, flat/proxy-dividend, rising/no-dividend, and multi-rebalance accounting checks passed; targeted suite **11 passed**.
- Full backend suite: **239 passed**.
- Python compileall passed.
- Vite production build passed.
- npm audit reported 0 high-severity vulnerabilities.
- Added-line security/dangerous-pattern scan and `git diff --check` passed.
- Independent review cycle 1 failed closed on missing proxy/non-PIT disclosure in saved-run history. A fresh fix agent added `ประมาณการปันผล (Proxy)` and per-row `descriptive non-PIT` badges. Fresh final review `deleg_10918fed` inspected the current post-hardening diff and returned `passed=true` with empty security, logic, and suggestion arrays.
### Known limitations
- `dividend_income` is still estimated from final holdings and final dividend yield; it is not a dated cash-flow ledger.
- The default public backtest still applies current scores historically, so `leakage_guard=false` and the result remains descriptive non-PIT.
- A supplied scorer toggles `leakage_guard=true`, but provenance and release-time semantics are not independently certified.
- Backtest run history remains in process memory.
### Exact next action
Implement dated dividend events and a provenance-validated PIT score provider before using results as strategy-performance evidence. Do not reset or stage unrelated pre-existing working-tree changes.
---
## Session 2026-08-25 — Multi-theme + Simulation (appended)
**Branch:** main · **HEAD:** aae1d13 (pushed `8a6991b..aae1d13`)

View File

@@ -169,7 +169,7 @@ Design (phased, honest about PIT):
| P0 | **Unify scoring** — registry-driven `compute_theme_surprises()` from `FACTORS`+`THEMES`; board calls it; delete parallel blocks | HIGH | board scores change → snapshot before/after; all 205 pass; sources unchanged | ✅ DONE (Option B: registry is source of truth; board re-ranked; parameters recentred) |
| P1 | **Simulation reuses board**`/api/v1/simulation` calls the same `RealDashboard` score (kill 3rd path) | MED | sim ordering == board ordering | ✅ DONE (verified live: sim picks PTT = top board) |
| P2 | **Source count clarity** — payload adds `factor_count` + per-factor detail; frontend shows "N ปัจจัย · M แหล่ง" | LOW | rendered shows both; 7≠5 confusion gone | ✅ DONE (`source_summary{factor_keys:11, rows:6}` live) |
| P3 | **Backtest PIT honesty** — real multi-rebalance engine, release-lag score_fn, actual rebalances, leakage flag, real-ish dividend | HIGH | 2024 backtest no longer uses 2026 scores; IC attribution added | ✅ DONE (`run_backtest` now reallocates every window; `leakage_guard`; `momentum_at` true 12-1) |
| P3 | **Backtest PIT honesty** — real multi-rebalance engine, release-lag score_fn, actual rebalances, leakage flag, real-ish dividend | HIGH | 2024 backtest no longer uses 2026 scores; IC attribution added | 🟡 Partial — multi-rebalance accounting invariants fixed; public endpoint still uses current scores historically (`leakage_guard=false`); dividend remains an explicitly disclosed final-holdings yield proxy |
| P4 | **Factor-weight learning loop** (user's new feature) — IC-based weight update + holdout | MED | a "before/after weight" is produced per factor per year | ✅ DONE (momentum IC=0.012 live; machinery ready — macro/demographic blocked on historical vintages) |
| P5 | **Hygiene** — split `__init__.py`, drop dead code, conftest/PYTHONPATH, registry-owned source metadata | LOW | diff is behavior-neutral | 🟡 Partial — dead code removed + conftest; `__init__` split + registry-owned metadata deferred |

View File

@@ -18,6 +18,7 @@
| Tourism deterministic signal | complete | live foreign-arrivals YoY surprise | add occupancy/airport metric |
| Internal paper ledger | complete | atomic local JSON persistence and restart test | shared store before multi-worker deployment |
| Dashboard | complete | Vite build + served source check with live-sign copy | visual browser capture after permission is available |
| General backtest accounting | complete/blocked | flat-price RED/GREEN invariants; 238 backend tests; Vite build; dividend proxy and non-PIT UI disclosures | replace final-holdings yield proxy with dated dividend cash flows; add provenance-validated PIT scorer |
| LLM analysis | deferred | intentionally no LLM dependency in M0 | add after signal lineage is stable |
| Webhook receiver | deferred | contract only, no external receiver | choose after core app is usable |
| MT5 bridge | deferred | not started | paper bridge after webhook decision |
@@ -96,3 +97,4 @@
- Audit + fix pass (2026-08-26, plan at `docs/audit-and-plan-2026-08-26.md`): triple-confirmed the declarative `FACTORS`/`THEMES` framework is by-passed by hand-written scoring in `dashboard._theme_surprises`, and that `/api/v1/simulation` recomputed a divergent 3-theme path. Fixed P1 (simulation reuses the canonical board via `default_scores` — live check: sim top pick PTT == top board combined 1.600), P2 (dashboard now emits `source_summary{factor_keys:11, rows:6}`; frontend shows "N ปัจจัย · M แหล่ง"), and P5-partial (removed dead `list_themes`/`Theme`/`build_theme_scores`/`_map_index` + the tests that locked them; added `backend/tests/conftest.py` so pytest needs no `PYTHONPATH`). Deferred P0 (registry-driven re-baseline) and P3/P4 (point-in-time backtest + factor-weight learning) pending explicit scope/baseline sign-off. Full backend suite: **203 tests pass**; Vite build passes. This work is own-engine review gated before commit.
- P0-B + P3 + P4 (commit `8db3d48`, reviews `deleg_fe6f45cd` + `deleg_718218f8` both `passed=true`): the declarative FACTORS/THEMES registry is now the single source of truth (`compute_theme_surprises` reads registry; hand-written per-theme blocks removed; FACTORS carries center/span normalization spec; bank NPL wired into banks). P3 rebuilt `run_backtest` as a real multi-rebalance PIT engine (`leakage_guard`, `momentum_at` true 12-1). P4 added factor-weight learning (Spearman IC -> `apply_weight_update`) + `/api/v1/learning/momentum`. Live result: momentum IC=0.012, t=0.132 over 22 periods (no reliable predictive power in this SET50 window). 226 tests pass.
- Follow-up (A+B): theme surprises now weight-normalized by total |weight| so cross-theme magnitudes are comparable (retail 0.189->0.145). Added append-only `FactorHistory` store (`data/factor_history/<key>.jsonl`) that records every FACTORS value each scheduler run, wired into `refresh_all` (non-fatal), plus `/api/v1/learning/factors` readiness endpoint. Macro/demographic factors start at n=1 and become learnable (P4) as history accumulates. 233 tests pass.
- Backtest accounting remediation (2026-08-27): deterministic flat-price tests exposed that ending holdings value was reported as `price_pnl` and the dividend proxy was excluded from final value/net return. The corrected identity is `final_value = capital + price_pnl + dividend_income`; API now emits `dividend_method=final_holdings_yield_proxy`, and both result/history UI paths label the proxy and warn when `leakage_guard=false`. RED failures reproduced `price_pnl=20,000/49,000`; GREEN verification: 239 backend tests, compileall, Vite build, npm audit 0 high-severity vulnerabilities, served-bundle disclosure check, static scan and diff check passed. Fresh final independent review `deleg_10918fed` passed with empty blocker arrays. Evidence: `docs/engineering-log/2026-08-27-backtest-accounting-remediation.md` and `docs/test-evidence/2026-08-27-backtest-accounting-remediation.md`. The dividend model remains a proxy and the default public backtest remains descriptive non-PIT.

View File

@@ -0,0 +1,69 @@
# Backtest Accounting Remediation — 2026-08-27
## Status
**complete** — core accounting implementation, disclosure remediation, local verification, and fresh current-diff independent review all passed.
## Scope
Correct the general multi-theme backtest result accounting without changing scoring, allocation, rebalance, price retrieval, or unrelated dirty working-tree files.
## Verified root cause
`backend/app/backtest.py` previously calculated:
- `final_value = ending cash + ending market value`
- `price_pnl = final_value - ending cash - dividend`
- `net_return = (final_value - initial capital) / initial capital`
This treated ending holdings value as price profit and excluded the dividend proxy from both final value and net return.
A deterministic flat-price reproduction with THB 100,000 produced `price_pnl=20,000` without dividends and `price_pnl=49,000` with a 2% dividend-yield proxy, when both price P&L values must be zero.
## Remediation
- Added flat-price regressions before production code changes and observed both fail for the expected accounting error.
- Defined ending equity before dividend as ending cash plus ending holdings market value.
- Defined price P&L as ending equity before dividend minus initial capital.
- Included the estimated dividend in final value and net return.
- Added API disclosure `dividend_method=final_holdings_yield_proxy`.
- Updated the Vue result panel to label dividends as estimates and show a non-PIT warning when `leakage_guard=false`.
## Changed files
- `backend/app/backtest.py`
- `backend/tests/test_backtest.py`
- `frontend/src/App.vue`
## Verification evidence
- RED: two focused regressions failed with `price_pnl=20,000` and `price_pnl=49,000`.
- GREEN: flat/no-dividend, flat/proxy-dividend, rising/no-dividend, and multi-rebalance accounting checks passed.
- Backtest module: 11 tests passed.
- Full backend suite: 239 tests passed.
- Python compileall: passed.
- Frontend Vite production build: passed.
- Fresh static server referenced `index-DlxBkioz.js`; all three new disclosure strings were present in the served bundle.
- Live API probe returned the corrected accounting fields including `dividend_method=final_holdings_yield_proxy` and `leakage_guard=false`.
- npm audit, high severity threshold: 0 vulnerabilities.
- Added-line static scan: no hardcoded secrets, shell injection, eval/exec, unsafe pickle, SQL formatting, or debug leftovers.
- `git diff --check`: passed.
- Independent review cycle 1 failed closed on one saved-history disclosure blocker. A fresh fix agent added per-history proxy and non-PIT disclosure. After test hardening, fresh final review `deleg_10918fed` approved the current scoped diff with `passed=true` and empty `security_concerns`, `logic_errors`, and `suggestions`.
Full command evidence: `docs/test-evidence/2026-08-27-backtest-accounting-remediation.md`.
## Remaining risks
- Dividend income remains an annualized proxy based on final holdings and final dividend yield. It is not a dated dividend cash-flow ledger and is disclosed as such.
- The public default backtest still uses current scores historically and remains descriptive non-PIT when `leakage_guard=false`.
- `leakage_guard=true` means a scorer callable was supplied; it does not independently certify release-time provenance.
- Existing backtest run history remains process-local memory.
- The in-app preview loaded the current application, but result-state visual capture could not be completed; fresh served-artifact and live API checks are recorded instead.
## Data safety and rollback
No database migration, external write, order dispatch, commit, or push occurred. Rollback is limited to the three changed code/test files listed above, but the repository contains unrelated pre-existing modifications that must not be reset wholesale.
## Exact next action
Implement a dated dividend cash-flow ledger and provenance-validated PIT scorer rather than extending the proxy.

View File

@@ -0,0 +1,140 @@
# Test Evidence — Backtest Accounting Remediation
**Date:** 2026-08-27 +07
**Branch:** `main`
**Base HEAD:** `b362cc3`
## RED evidence
Command:
```text
PYTHONPATH=. python3 -m pytest -q tests/test_backtest.py::RunBacktestTest::test_flat_price_has_zero_price_pnl tests/test_backtest.py::RunBacktestTest::test_dividend_is_included_in_final_value_and_net_return
```
Expected failures observed:
```text
2 failed
flat/no-dividend: price_pnl 20000.0 != 0.0
flat/2%-proxy: price_pnl 49000.0 != 0.0
```
The failures were caused by the production accounting bug, not test syntax or setup.
A second RED cycle for API disclosure failed with:
```text
KeyError: 'dividend_method'
```
Test hardening later added a rising-price/no-dividend case and an explicit multi-rebalance accounting identity. The first draft incorrectly expected all capital in the non-dividend name and failed `4,000 != 10,000`; canonical allocation intentionally assigns non-dividend names to the 20% bucket, so the expected P&L was corrected to 4,000 without changing production code.
## GREEN evidence
Focused regressions:
```text
2 passed
```
Backtest module:
```text
11 passed
```
Full backend suite:
```text
239 passed
```
Python compilation:
```text
python3 -m compileall -q backend/app backend/tests
passed
```
Frontend production build:
```text
vite v6.4.3 building for production
10 modules transformed
built in 352ms
```
Fresh served artifact verification:
```text
served_chunk=index-DlxBkioz.js
served_ui_strings=present
```
The fresh served bundle contains `ประมาณการปันผล`, `descriptive non-PIT`, and `ไม่ใช่หลักฐานประสิทธิภาพกลยุทธ์`.
Live API probe through the Vite frontend proxy:
```text
final_value=1,146,565.40
price_pnl=94,271.57
dividend_income=52,293.83
dividend_method=final_holdings_yield_proxy
net_return=0.1466
leakage_guard=false
```
The returned values satisfy the accounting identity after response rounding.
Dependency audit:
```text
npm audit --omit=dev --audit-level=high
found 0 vulnerabilities
```
Static added-line scan:
```text
hardcoded secrets: none
shell injection: none
eval/exec: none
unsafe pickle: none
SQL string formatting: none
debug leftovers: none
```
Diff integrity:
```text
git diff --check
passed
```
## Deterministic accounting invariant
For flat prices and no dividend:
```text
price_pnl = 0
final_value = initial capital
net_return = 0
```
For flat prices and a final-holdings yield proxy:
```text
price_pnl = 0
final_value = capital + dividend_income
net_return = dividend_income / capital
final_value = capital + price_pnl + dividend_income
```
## Independent review gate
Cycle 1 failed closed with no security finding and one disclosure logic error: the newly executed result card was honest, but the saved-run history table still labelled the proxy as ordinary dividends and displayed `leakage_guard=false` rows without a visible non-PIT marker. A fresh fix agent changed only `frontend/src/App.vue`: the history column is now `ประมาณการปันผล (Proxy)`, and each `leakage_guard=false` row gets a `descriptive non-PIT` warning badge. Frontend build and `git diff --check` passed after the fix; the freshly served production chunk `index-CzZTebYa.js` contains both disclosures. Fresh final review `deleg_10918fed` inspected the current post-hardening diff and returned schema-valid `passed=true` with empty security, logic, and suggestion arrays. The earlier failed and stale verdicts are not treated as approval.
## Manual/live scope
No live order, webhook, broker, external mutation, commit, or push was performed. The in-app preview loaded the current page, but the result-state interaction did not produce a capturable UI state. Visual result-state capture is therefore not claimed; verification uses the fresh served chunk plus the live API contract.