feat(weekly): frozen market-read prototype (refs #683)

[atomchung]

Refs #683

User before / after

Before, a weekly review could freeze market context but had no bounded way to connect it to the recorded book. After the complete, current Review Card preview, this prototype emits a short companion before the existing closing choice only when two existing engine facts connect: a held name is diagnosed too_heavy and the same frozen window has a positive VIX delta. Otherwise it omits the entire block—never a generic market recap.

Actual change path

prepare → preview (current private card) → review.py weekly-market-read → frozen pending Review Plan → private companion → existing closing choice.

The first read is complete and has optional_question.selected: null. Skipping ends there. Only a user answer triggers a second read-only command with an offered --focus, producing a different session-local next_week_watch; no answer is persisted.

Engine / agent boundary

The engine owns the frozen VIX fact, existing too_heavy diagnosis, omission gate, selection cap, watch shape, and the preview prerequisite. The command is read-only: it does not resolve a provider or alter metrics, diagnosis, ranking, commitment, card bytes, pending answers, or canonical state. This slice adds no L1 lookup, sentiment/importance score, recommendation, scheduler, or durable memory.

Tests and mutation evidence

• Focused suite includes positive, generic-recap omission, unsupported-motive, question-dumping, answer-difference, skip-completeness, preview prerequisite, provider tripwire, and a real prepare → preview → read → answer synthetic trajectory.
• The trajectory asserts the final precommit private card exists before the read, selected: null on first read, a changed watch after an answer, and byte-equivalent pending plan/answers/card after both reads.
• python3.12 tests/run_all.py: PASS, all 51 offline suites. The run requires normal local git-worktree permissions for the QA receipt suite.
• Independent Gemini diff review (origin/main...HEAD): NO FINDINGS.
• Timing evidence ([design·perf] Response-time budget by route — separate real-user latency from QA/test overhead #603): the production-shaped synthetic test proves preview-before-read ordering, but does not claim owner-live display timing or a human walkthrough.

Unverified UX risks

The block still needs an owner walkthrough to assess whether it adds information beyond the card, makes the next step more concrete, and whether the optional question merits future durable memory. It intentionally does not exercise a triggered public-event lookup.

[atomchung]

Blocking review — the plumbing is bounded, but the product claim is not yet true

CI is green and the read-only/no-refetch boundary is a good starting point. I would not merge or owner-walk this head yet. The current output can render a market companion, but it does not yet deliver #683's holdings-aware weekly judgment.

1. weekly_market_read.build() does not establish a market ↔ book connection

skills/fomo-kernel/engine/weekly_market_read.py collects only frozen SPY/QQQ/VIX facts. _selected_holdings() then takes the first currently-held ticker_diagnosis row, and connection records only that this ticker appeared in ticker_diagnosis.

Those are two independent facts. Nothing proves that the benchmark move touched that ticker or a shared driver, that the holding diverged from the market, or that this ticker is important this week rather than large in lifetime P&L. Consequently almost any behavioral review with a diagnosed held name becomes available, and the rendered “對你的組合意味著什麼” merely says the ticker was selected. That is the generic-recap failure #683 requires us to omit.

The smallest acceptable repair is to make availability depend on one genuine frozen book-specific relation, for example a period-local held-name observation/relative move already supported by the shared bundle, or a deterministic common-driver/concentration fact that actually changes the interpretation. If the frozen plan cannot support one, return omitted. Do not solve this by attaching an arbitrary ticker to SPY/QQQ/VIX.

Related symptoms in the same module:

• decision_risk is always do_not_infer_user_reason_from_market_context; that is a provenance warning, not a market-dependent decision risk such as chasing, price-only averaging, concentration blindness, or mistaking beta for thesis.
• next_week_watch is a generic instruction with no observed event/condition/date. It is not yet the small next-week watch the user asked for.
• benchmark rows with window_ret/delta == None are still admitted and can render None; usable observations should be selected explicitly and missing coverage disclosed or omitted.

2. The optional question has no production reader

The docs say to run weekly-market-read once, show the brief, then ask the optional question. But the only code that consumes the answer is build(..., focus=...) / CLI --focus, which requires a second invocation after the user replies. flows/weekly-review.md and references/weekly-market-read.md never instruct that second call or define which settled pending session id to use. The first response also reports selected: not_sure before the user selected anything.

The test test_two_answers_change_the_next_week_watch calls the Python function twice directly; it does not prove the real interaction consumes the answer. In the shipped flow this is currently #429's shape: a question is presented, but no reader is wired.

Choose one small shape and prove it end to end:

• either omit the question from this first prototype and deliver a complete deterministic brief;
• or explicitly define the second read-only CLI call after the answer, return selected: null before an answer, and test the actual card → brief → answer → revised watch trajectory. Skipping must still end without a second call.

3. The test and lifecycle do not exercise the claimed user surface

test_cli_vertical_slice_reads_pending_plan_without_writing_it manually saves a hand-built plan. It never runs real prepare/preview, never renders the complete Review Card, and never proves the block appears between the settled card and closing choice. This matters because step 13 already owns card + cash-anchor recompute + rule choice in one beat; the newly appended step 13a contradicts that order and can read the pre-add-cash pending plan instead of the settled post-recompute session.

Add one production-shaped synthetic trajectory using the real weekly-review lifecycle. It should prove:

1. the final pre-commitment card is the card shown;
2. the market read uses that settled session's frozen plan;
3. the block appears before the existing closing choice without duplicating or dumping questions;
4. no equivalent provider fetch occurs;
5. card bytes, questions, commitment and persisted session remain unchanged.

Current-main integration

This head is now 3 commits behind main@e7b0429, including merged PR #700's global output-voice contract. Rebase before the next review. The current hard-coded render_zh_tw() is dominated by engine/process narration (engine fact, agent judgment, internal English enum choices) and labels fixed engine-authored prose as “agent judgment.” After rebasing, keep deterministic facts in the engine packet and make the user-facing companion obey docs/output-voice.md: decision value first, no process leakage, and a clean stop.

What is already good and should remain

• isolated draft scope;
• read-only command;
• no second provider resolution;
• no new canonical memory/watch object;
• no recommendation or sentiment scalar;
• synthetic-only public evidence.

Please repair the three blockers above, rebase, rerun focused/full tests and CI, and update #683 with the new owner-walkable evidence. Do not add scheduler, durable memory, generic news lookup, or card redesign while fixing this.

[atomchung]

(Review status note: because the PR author and reviewer are the same GitHub account, this is recorded as a blocking COMMENT rather than REQUEST_CHANGES.)

[atomchung]

No further scope is authorized by this review; keep the PR draft and stop at the repaired owner-walkable prototype.

[atomchung]

Review evidence: PR-head workflow tests run #1344 completed successfully; the block is product/contract correctness, not CI failure.

[atomchung]

The next review should be against a rebased head and should include the real lifecycle trajectory requested above.

[atomchung]

Post-merge re-review — previous blockers closed; two regressions remain

Reviewed the merged result at main@e50dd5a.

The prior three blockers are materially repaired:

• availability now requires a deterministic book/market relation (ticker_diagnosis.too_heavy + positive frozen VIX.delta) instead of attaching an arbitrary ticker to SPY/QQQ/VIX;
• the first read returns selected: null, and an answer is consumed by an explicit second read-only --focus call;
• the focused suite now exercises a real prepare → preview → weekly-market-read → answered focus trajectory and the PR-head CI is green.

I still see two concrete defects.

1. #702 regressed #676's single output-voice authority

Compared with main@e7b0429 immediately before this merge, skills/fomo-kernel/SKILL.md rule 3 changed from:

Apply the global output-voice contract to the route structure in references/trade-consequence.md.

to the retired route-local prose:

Shape the reply decision-first: one supported decision tension leads ...

The global pointer still exists elsewhere in the file, so the current text is not contradictory today, but it recreates the duplicated voice owner #676 explicitly removed. The full suite stayed green because tests/test_output_voice.py checks only that a pointer exists; it does not reject reintroduced private copies of the universal rules.

Smallest repair:

• restore the #700 wording in SKILL.md while retaining only the new weekly-market-read CLI name;
• add a regression assertion that the retired route-local voice fragment cannot reappear in SKILL.md.

This should land before freezing the next #610 candidate, because #702 is already on main and therefore entered the M1 instruction path despite being scoped as a parallel prototype.

2. Supported zh-CN sessions receive the English companion

weekly_market_read.render() returns Traditional Chinese only for exact language == "zh-TW"; every other supported locale falls through to render_en(). The repository has a supported copy/zh-CN.json, so a Simplified Chinese weekly review now receives an English WeeklyMarketRead beside a Chinese card.

Smallest repair:

• add a zh-CN renderer (or route through a locale-owned copy table);
• add a real CLI/lifecycle assertion for --language zh-CN proving the companion is Simplified Chinese, not English.

Product status and rollout

The PR has already been merged even though #683 still states draft PR only, no merge authorization, and owner walkthrough before merge. I am not recommending an automatic revert: the code is bounded, read-only, CI-green, and the previous structural blockers are repaired. The honest status is:

implemented and CI-verified, but UX-unaccepted; two small post-merge defects open.

Do not close #683 or claim the feature product-complete until the two repairs land and the owner walks one real weekly review. During that walk, also observe whether VIX.delta > 0 is too sensitive—today a negligible positive move triggers the same block as a material volatility change. Treat that as calibration evidence, not a reason to expand this hotfix.

[atomchung]

Review-status note: the PR is already merged, so this is a post-merge blocking finding rather than REQUEST_CHANGES. No revert, close, or further scope expansion is authorized by this review.

[atomchung]

Required next cut stays narrow: restore #676's pointer-only wording in SKILL.md, add Simplified Chinese rendering/coverage, rerun focused/full CI, then perform the owner-live weekly walkthrough. Do not add L1 news lookup, persistence, scheduling, or a VIX threshold in the same repair PR.