Files
set50-system/docs/audit-and-plan-2026-08-26.md
Kunthawat Greethong 325e164dd3 [verified] Fix P1-P2-P5 audit findings: simulation reuses board, source_summary clarity, dead-code removal + conftest
- P1: /api/v1/simulation now uses the canonical board score (default_scores)
  instead of a divergent 3-theme recompute -> 'จำลอง' can't disagree with board
  (live check: sim top pick PTT == top board combined 1.600). Removes binary
  auto/en signs, restores quality+momentum+dividend screen consistency.
- P2: dashboard emits source_summary{factor_keys, rows}; frontend shows
  'N ปัจจัย · M แหล่ง' so the 7-vs-5 count confusion is impossible.
- P5: removed dead themes.list_themes()/Theme/build_theme_scores/_map_index and
  the tests that locked them; added tests/conftest.py so pytest needs no PYTHONPATH.
- docs: audit-and-plan-2026-08-26.md (full P0-P5 plan) + engineering-log entry.
- 203 backend tests pass; Vite build passes. Independent reviewer: no security or
  logic blockers (minor error-leak suggestion applied: 503 message no longer leaks
  exception detail).
2026-08-27 03:21:22 +07:00

13 KiB
Raw Blame History

Audit + Implementation Plan — SET50 Alternative Data Platform

Date: 2026-08-26 · Branch: main (clean tree, @ 6e78b6a) Scope: 4 questions (dead logic / backend↔frontend contract / methodology / code hygiene) + new point-in-time backtest feature for factor-weight learning.


0. Executive summary

The app works and 205 tests pass (PYTHONPATH=. python -m pytest tests -q). But the architectural core has a real flaw: a declarative FACTORS/THEMES framework was built, yet the actual scoring by-passes it with hand-written blocks in dashboard._theme_surprises. The registry only drives the sources table; it does not drive the analysis. Every new collector needs manual scoring code — the exact thing factors.py claims to have eliminated. This is the root cause behind nearly every finding below, including the source-count confusion and the hard-to-maintain scoring.

Findings triple-confirmed by 3 independent read-only subagent audits (2026-08-26) plus my own reading. Exact counts established: 11 FACTORS entries → 5 fetch-module groups → 6 rendered source rows; no code path yields 7 (the user's "7" is registry entries labeled BOT / category conflation).


1. Q1 — Built-but-unused / misaligned code

1.1 DEAD METADATA: the declarative factor framework is not what computes scores [HIGH]

  • factors.py: FACTORS, _FETCH_MODULE, factor_value(), normalize(), z_score()
  • themes.py: THEMES (dict w/ per-theme factor weights), list_themes(), Theme dataclass
  • Evidence: grep shows factor_value, normalize, z_score, list_themes are referenced nowhere outside their own definitions. Nothing iterates THEMES to compute a surprise (grep THEMES in app/*.py → only the definition). factor_value is dead. The weight/sign/ value_key fields in FACTORS are never consumed by scoring — only the sources table (dashboard._build_sources) reads FACTORS for label/source/frequency.
  • Purpose misalignment: factors.py module docstring promises "ADDING A FACTOR … requires NO change to any scoring function." False in practice — see 1.2.

1.2 TWO parallel scoring systems (the big one) [HIGH]

The real analysis lives in hand-written code:

  • dashboard._theme_surprises (z-scores + _norm(...) per theme, all inline)
  • dashboard._build_boardthemes.build_siamchart_score + combine_score + quality_within_theme
  • The THEMES factor weights (e.g. auto NPL weight 0.6, banks macro_investment 1.0) are ignored; the effective weights are the inline math in _theme_surprises (e.g. auto: new_car_sales_yoy then npl subtract block).
  • Consequence: editing a factor's weight in THEMES/FACTORS does nothing; you must edit the hand-written block. Docs and symbol_breakdown's "transparent" notes therefore can drift from truth.

1.3 /api/v1/simulation recomputes a THIRD, different scoring path [HIGH]

  • __init__.py:567-662 re-derives scores inline: only 3 themes, auto_sign/en_sign = binary ±1 (not the real surprise), no quality_within_theme, no momentum (R2), no dividend screen (R5). It even pulls its own auto_credit/energy_thai cache keys instead of the board.
  • Result: the "จำลอง" (simulation) and "Backtest" panes can disagree with the "ตารางหุ้น" board on which names rank highest — same bug class as the frontend mismatch the user is worried about, but inside the backend. This is the strongest "doesn't match its purpose" find.

1.4 bank_npl IS wired into banks score — but via hand-code, not the registry [MEDIUM]

  • Not dead: dashboard.py:276-281 blending it into banks_s is live (I initially flagged it dead, then verified it is reached). It does affect the banks surprise and hence the board.
  • But the blend ignores FACTORS['bank_npl']['weight']/sign/value_key — it hardcodes (bnpl - 0.5)/3.0 capped at 0.5. If bank_npl were removed from FACTORS, the sources row would drop but the scoring block would silently keep running (and vice-versa). Registry ↔ scoring are unlinked.

1.5 Minor dead/vestigial code [LOW]

  • themes.list_themes() + Theme dataclass: unused (legacy of the 3-theme era).
  • backtest.BacktestResult.rebalances is set to len(dates) but the loop breaks after the first date → the number is misleading (see 3.1).
  • simulation.load_price_snapshot() vs backtest's own loader — two snapshot loaders; verify they read the same file family.

2. Q2 — Backend↔frontend contract

2.1 Source count: NOT hardcoded — already derived [INFO — corrects the assumption]

  • Frontend count = dashboardSources.value.length (App.vue:54) → purely derived from backend sources array. No hardcoded "5" or "7" anywhere.
  • Backend _build_sources derives rows from FACTORS grouped by fetch module → currently 6 unique source rows (7 factors → 6 rows because auto_npl+bank_npl share BOT FI_NP_003_S2, and macro_* collapse under macro_thai).
  • "7 vs 5" = the user is counting differently (7 factor keys vs 6 distinct providers vs the 20+ thing). The right fix is to make the displayed count unambiguous — show "N ปัจจัย · M แหล่ง" and add a factor-level detail, so the number can't be misread.

2.2 Real contract gaps (what to verify at runtime, not trust) [MEDIUM]

  • Frontend renders s['จาก'], s['แหล่ง'], s['ข้อมูล'], s['ความถี่'], s['อัปเดตครั้งต่อไป'], s['dึงมาเมื่อ']. The backend emits these exact Thai keys — good, but the v-for row uses key="i" (index) — fine here since static, but brittle if sources reorder/refresh.
  • Frontend's themes/combinedBoard come from /api/v1/themes; board from /api/v1/dashboard; simulation from /api/v1/simulation — three endpoints that each recompute instead of sharing one canonical payload. The board and simulation can diverge (see 1.3).
  • Verification still needed: the last rendered-frontend check was against a stale/refused Vite (5173). The contract is derived-correct now, but I will re-render to confirm rows/columns show 6 and the board == simulation ordering.
  • Make /api/v1/simulation and /api/v1/backtest call the same RealDashboard.build() board (pass a score_fn that returns the live board) instead of re-deriving. One source of truth.

3. Q3 — Methodology: can the analysis improve? (incl. the new backtest feature)

3.1 Current backtest is NOT point-in-time — it leaks the future [CRITICAL]

  • backtest.run_backtest uses default_scores(syms) = today's combined score for every rebalance date, including 2024. So a 2024→2026 backtest "invests in 2024" using 2026 knowledge (price, fundamentals, theme surprises). Sign is therefore meaningless as evidence.
  • rebalances counts all dates but it allocates once then holds (break at line 167). Mislabeled.
  • Dividend = cost × current_yield — a rough proxy, not a real per-period payment.

3.2 What the user's new feature needs (the Dec 10 → Nov 30 example)

Goal: learn factor weights from history — e.g. does a rising "children/population" factor lift the relevant SET50 names the following year? If yes, weight that factor up.

Design (phased, honest about PIT):

  1. Vintages / point-in-time factor series. For each alternative factor, keep the as-published series with a published_at (release date), not just the latest value. (The repo already has a vintages store + event_study infra — reuse that pattern, which already guards require_price_known_at.)
  2. Factor→forward-return attribution. At each monthly rebalance t, using only data published ≤ t - lag (e.g. lag = release cadence), rank symbols by each factor's z-score; measure the forward 112m return of the top-minus-bottom spread. Aggregate IC (information coefficient) per factor across the window.
  3. Weight update rule (the user's ask). Start from current weights; after each validation year, raise/lower a factor weight proportional to its realized predictive IC (e.g. w' = w * (1 + shrink * IC)), clamped to sane bounds. This is the "if population up → children stocks up → weight up" learning loop, made explicit and testable.
  4. Honesty guardrails:
    • apply release-date lag (no lookahead by construction)
    • keep a holdout year untouched by the weight learning to avoid overfit
    • report IC spread with confidence (n symbols, EM is noisy)
    • the multi-rebalance engine must actually rebalance (sell/re-buy), not allocate-once
    • real dividend handling per payout date (or clearly call it a proxy)

3.3 Methodology improvements worth doing (independent of the backtest)

  • Unify scoring: one function computes theme surprise from FACTORS+THEMES registry (kill the parallel hand-code). This alone removes the biggest correctness risk.
  • Quantile/IC instead of absolute z: the regime gate and LONG/SHORT bars already use quartiles; apply the same normalization consistently to components.
  • Model the "children/population" style factor properly: it's a slow-moving thematic tilt, not a high-frequency surprise. It should enter as a structural theme weight / industry tilt (longer horizon), not as a monthly z-score — otherwise it will rank as noise.
  • Frequency-honest mixing: THEME_FREQUENCY is declared but never enforced; factor values of different cadence are combined as if same-as_of. Align to the slowest, or carry real as_of.

4. Q4 — Code hygiene / clarity (no behavior change)

  • __init__.py is 804 lines — split route handlers into a routes/ package (or at least pull the three big recompute endpoints into one build_dashboard_payload()).
  • dashboard.py and themes.py both define surprise math; consolidate into themes.py registry-driven scorer; dashboard.py keeps only assembly.
  • Remove dead exports: list_themes(), Theme, factors.factor_value/normalize/z_score if unused after unification (keep whatever becomes consumed).
  • backtest.py: rename rebalancesplanned_rebalances, add actual_rebalances; add leakage_guard flag that is False until a PIT score_fn is supplied.
  • _build_sources: the in-function _source_label / _cadence dicts are fine but should move to the registry (each FACTOR carries its own source_label, cadence) so sources can't drift from factors.
  • Tests: currently must run with PYTHONPATH=. (ModuleNotFoundError: app otherwise) → add tests/conftest.py inserting the backend dir, and a CI-safe pytest config. 205 tests pass but none cover: backtest look-ahead guard, sources-count derivation, or the simulation↔board consistency.

5. Proposed implementation order (each step independently shippable + tested)

# Change Risk Acceptance Status
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 Deferred — needs baseline sign-off (would re-baseline live scores)
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 Deferred (needs scope decision)
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 Deferred (needs scope decision)
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

Canonical repo conventions: plan-first, fix only reported bug, log to engineering-log.md + HANDOFF.md per session (engineering-handoff skill), push WIP early to gitea origin.


6. Open decisions for the user

  1. Scope now: do I execute P0P2 (correctness + contract) in this pass, and leave P3/P4 (the point-in-time backtest + weight learning) as a separately planned feature? Or attempt the full stack?
  2. Backtest horizon feature (P4): the "children/population" factor — implement as a long-horizon thematic tilt weight (my recommendation) vs a monthly surprise?
  3. Acceptable score change: P0 will shift board scores (registry weights may differ from today's hand-math). OK to re-baseline, or must output stay bit-identical (forcing registry weights = current hand-math)?