Repository navigation
fix(desk): complete S2 desk observation, freshness and next-run fixes - #1171
Conversation
Completion round for the SAME-S2 Desk corrections (review of #1170). - Performance: the last qualified day's observation is the chronologically newest normalized instant, not the lexically last string; a missing or unusable instant still leaves no time basis. - Overview: displayed-observation freshness no longer depends on aggregatability. Mixed-currency cohorts keep unavailable totals (no FX) but report the displayed facts' age. The degraded golden changes accordingly. - Logs: the next-run line keeps status, coverage, the stored observation instant and stale age; a passed candidate expires on a local clock; times carry their UTC date across UTC midnight. The page keeps one ribbon read per open and makes no added read. - TopBar: a next run on another UTC day shows its UTC date. - Contract: correct the UTC-day/NY-day overlap statement and make the todaySummary compatibility claim precise (shape unchanged, values intentionally corrected). Comment-only change in operations.js. The BenchmarkPanel state item is not included: it needs a PerformancePage.tsx change outside the approved binding. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
A live Performance page never reads the benchmark source, yet it reported that intentional non-read as "failed" (fetch-failed copy beside the contained copy) and briefly as "loading". - Add a `not_requested` BenchmarkState; loadBenchmarks returns it when the source allows no read, without calling the API. - The page starts in `not_requested` and sets `loading` only when a read will be made; PerformanceView shows a disallowed source as `not_requested` whatever state it holds. - BenchmarkPanel renders `not_requested` with the contained copy only. - Tests cover live/mock phases, both languages, and the unchanged zero-call and single api.benchmarks( pins. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Independent completion-round verdict: PASS WITH RESIDUALS. Blocking findings: 0. QuantStrategyLab/QuantRuntimeSettings, PR #1171, branch This is an independent session review, not an official Codex GitHub App receipt or owner approval. All six items pass independent code review:
Scope/build evidence: exactly 18 changed files, all bound; complete non-generated diff read. Operations/ribbon changes are comment-only. Independent clean frontend build and sync reproduce all 45 assets, HTML and build identifier byte-for-byte; the asset map is 1,344,219 bytes. No schema, lockfile, new source integration, flow wiring or execution authority change. Original checkout clean before/after; no remote or production writes/reads. Prove-it-fails: five isolated reversions in a scratch tree fail their new assertions: performance selector, benchmark disallowed-load result, direct held-state rendering, loading initialization and the old panel. Restoring final code passes all three affected regression scripts. The initialization check is a source pin; the other four fail behavioral/render assertions. Executed checks: all five bound test files; full console Environmental failure: deploy-safety fails identically at head and the requested main base because the shallow clone lacks a pinned historical release commit; Git ancestry exits 128. Full-history hosted CI must clear it normally. No test was weakened or gate bypassed. Initial scratch-only missing-Git and missing-dist setup failures were resolved and are retained in the report. Residual dispositions: c1's helper placement, stricter direct-input timestamp qualification, fixed snapshot age, separate Logs/shell reads and Chinese stale wording are accepted limitations; Overview has no next-run display, so no missing change exists there. c2-R1 is the confirmed environment failure. c2-R2 is accepted: unavailable rows/KPI with explicit “comparison is not read” are nonblocking and invent no values. The broader no-data footer remains optional wording cleanup, not proof about remote data availability. Logs' 30-second local tick is approximate, not an exact scheduling guarantee. None of these is silently called fixed. The refreshed next-batch proposal treats 7A/8A/9A/12A/NA as approved design direction, not outstanding choices and not implementation authorization. It retains all class-C prerequisites (#14–17, #20/#24, #22, #25, #26), the open copy/maintenance/snapshot limits, production evidence gaps and separately scoped timestamp investigations. Class-D risk/analytics/alerts/execution work stays deferred. The head is ready for the owner's merge decision based on this code review. Actual merge eligibility remains subject to current-head required hosted checks and the official review receipt; neither was re-read here. This document grants no merge, deployment, collection, notification or trading authority. Nothing was posted. Full report: |
Owner approval — merge authorization (S2 completion round, PR #1171)The account owner approved this merge in the Muse conversation at 2026-11-14T21:39:39+08:00 (Asia/Singapore), replying “批,合併 PR #1171” to the coordinator's merge request. Approved for merge:
The coordinator posts this record on the owner's behalf and will add the |
|
@codex review |
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
AI assistance
Claude Code, model Claude Opus 5.5 (
claude-opus-5-5), acting as the single implementing writer for the owner-approved SAME-S2 completion round (2026-11-14).What changed
Five of the six code-only residuals from the end-of-batch review of #1170:
application/desk/performance.js). The last qualified day'sobserved_atis the chronologically newest normalized instant (epoch comparison), notlastTimes.sort().at(-1). A missing or unusable instant on that day still leaves no time basis, and equal instants normalize to the same text. Regressions cover whole, fractional and offset instants in both member orders, equity plus the four risk wrappers, the stale-age boundary (345599 s vs 345600 s), partial history by inception and by retention, null, undefined and unusable observations, and a null-flow control showing thathistoryandreturn_basisare unchanged.pages/LogsPage.tsx). The next-run line keeps the whole read: the qualified headline "earliest observed scheduled run", then coverage, the stored observation instant and, when stale, the reported age. Unknown names its reason. A null value reads "not observed", never "none due". A candidate whose due instant has passed on the local clock shows as passed. The page keeps its singleapi.ribbon()read per open: a one-time snapshot that is never polled and adds no storage read. The page says this on screen. The local clock is anchored to the snapshot'sgenerated_atonly to place "now". It never refreshes an observation or its age. Every time on the page carries its UTC date when that date is not the current UTC day. Rendering is split into a pureLogsViewso it can be tested.application/desk/overview.js). Freshness is gated on displayed fresh, bound, non-paper observations, not on the totals coverage. Mixed currencies keep unavailable totals and no FX, but now report their displayed age.n1-overview.degraded.jsonwas regenerated through the test's own writer; theokandpartialgoldens are unchanged.shell/TopBar.tsx, contract,ribbon.jscomment). A next run on another UTC day than the viewer's shows its UTC date. Coverage and stale age are kept. The contract no longer claims that a past?date=never reads today's record: until New York midnight, yesterday's UTC slice still overlaps the current New York business day. No expected or pending count is derived.operations.jscomment, runtime-today test comment). The defaulttodaySummaryoutput shape is unchanged and has noschedulekey. The approved completeness and chronological corrections intentionally change values that were wrong before. Comment and wording only; one test pins that the opt-in adds only theschedulekey.Not included: BenchmarkPanel state honesty. Telling "not requested" apart from "request failed", and dropping the loading state on the live path, needs a change in
pages/PerformancePage.tsx:loadBenchmarksreturns{phase: "failed"}when the read is not allowed, and the page sets{phase: "loading"}before it. That file is approved only for item 1 display consistency, so this item is held pending a binding exception.How verified
node tests/desk_n1_projection_validation.mjs: passnode --experimental-strip-types tests/console_desk_n1_validation.mjs: passnode --experimental-strip-types tests/console_desk_phase8_validation.mjs: passnode --experimental-strip-types tests/console_desk_benchmarks_validation.mjs: pass (unchanged)node tests/runtime_today_incomplete_validation.mjs: passnpm run test:operations --prefix web/strategy-switch-console: pass (exit 0)npm run build(frontend: tsc, vite, size budget): pass;npm run sync:v2-assetsregeneratedv2_asset_map.jsnode --experimental-strip-types tests/console_v2_assets_validation.mjs(45 assets) andtests/console_v2_worker_validation.mjs: passpython3 scripts/check_style.py: 0 blocking, 1 advisory (README outline; the same advisory appears on the base)build_config.py --check,build_display_names.py --check,runtime_settings.py validate: passoverview.js.worker.jsis hand-maintained source that imports the application modules, so the build did not change it.Why
The independent review of #1170 found these residuals:
Risk
Low. Read-only Desk projections and presentation, synthetic fixtures and tests only. No schema, package, lockfile, data source, scheduling, storage-read, write or deployment change. Values intentionally change in two places: the mixed-currency Overview
freshness/data_age_seconds(now the displayed age instead of unavailable) and the Performanceobserved_atwhen instants within one second differ in fractional digits or offsets. No labels were added; risk classification is left to the normal gate.🤖 Generated with Claude Code