Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 85 additions & 0 deletions PHASE_STATUS_INFLIGHT.md
Original file line number Diff line number Diff line change
Expand Up @@ -3867,3 +3867,88 @@ x-position (x9), same fill, rounded-sm; screenshot confirms the chevron box now
matches the nav-item box. `tsc` + `next build` (506) clean.

---

## Stability bug-fix bundle — sidebar refresh/rotate flash + chart crosshair re-park + 2 flaky-test guards (in flight, 2026-05-30)

Three stability fixes on `claude/optimistic-brown-UUcXA`, all post-#325. Two
are the user-reported sidebar/chart bugs; two are flaky-test hardening
("เก็บ flaky ให้ครบ"). Frontend + test-only — zero schema / compute / scoring /
valuation / output-JSON change.

**Bug A — "sidebar opens then shrinks back by itself" on refresh OR
portrait↔landscape rotation.** Two root causes, two-part fix:
- The static export bakes the EXPANDED rail into every page's HTML, so on
refresh the rail painted wide, then `AppShell`'s mount effect read
`localStorage['quantrank.sidebar.collapsed']` and collapsed it — a visible
width shrink. Fix: a pre-paint inline `<script>` in `layout.tsx` adds
`.sidebar-collapsed` to `<html>` BEFORE the body paints (mirrors next-themes'
dark-mode pre-paint), and a new `globals.css` rule
(`@media (min-width:768px) html.sidebar-collapsed aside[data-sidebar-rail]`)
renders the rail at its collapsed 4rem width immediately → React's
post-hydration collapse is a no-op. `AppShell` keeps the class in lockstep
with the live state (removed on expand) so the rule never fights React.
- The aside carried a permanent `width 200ms` transition, so the
hydration-collapse AND the breakpoint cross on rotation both ANIMATED the
shrink. Fix: a new `animate` flag (owned by `AppShell`, passed to `Sidebar`)
is true only for ~250ms around an EXPLICIT user toggle (collapse chevron /
mobile hamburger / backdrop); at rest the aside is `transition-none`, so
refresh + rotation + resize switch width/position INSTANTLY (no shrink
motion). Explicit toggles still animate smoothly.

**Bug B — chart crosshair jumps to the far LEFT when the sidebar is
expanded/collapsed.** Toggling the rail reflows the main-content width →
`ResponsiveContainer` re-measures the chart, but Recharts applies `defaultIndex`
(latest-point park) only on MOUNT, so the crosshair drifted to a stale/left x.
Fix (`PriceHistoryChart.tsx`): replaced the orientation-only `matchMedia`
re-park with a width-delta `ResizeObserver` on the chart wrapper that
debounce-bumps the remount key (`layoutKey`) ~300ms after any width change
settles → `<AreaChart>` remounts → re-parks at the latest point AFTER the
re-measure. Subsumes the old orientation listener (rotation changes width too)
and now also catches sidebar toggle + window resize. A width-only delta gate
(`<1px → ignore`) prevents height-only / crosshair-render churn from triggering
spurious remounts. Empirically: cursor x-ratio before=1.0, transient@120ms=0.0
(the bug), after-repark=1.0 (fixed).

**Flaky-test hardening (เก็บ flaky ให้ครบ):**
- `test_ranking_history.py::test_load_ranking_history_smoke_recent_window` —
added an explicit `if df.empty: pytest.skip(...)` guard (mirrors the
`list_ranking_commits` guard from #325 + PR #284). On a shallow CI clone
(`actions/checkout` fetch-depth=1) `rankings.json` history may be unreachable
→ empty frame; the populated-MultiIndex-shape assertions need a full clone.
The test previously passed-on-empty only by coincidence of the empty-return
path's `.set_index(...)`; the guard makes the shallow-safety explicit +
edit-proof (proven via a real depth-1 clone: 46/46 git-dependent tests pass).
- `test_osap.py::test_package_imports_and_exposes_openap_class` — was a
default-lane (non-`@network`) test that instantiated
`openassetpricing.OpenAP()`, whose 0.0.2 constructor does a LIVE Google-Drive
metadata fetch → flaky whenever Drive rate-limits (returns a "Quota exceeded"
HTML page; polars then raises `ColumnNotFoundError` on the missing "Acronym"
column — hit live in this session's pre-push gate). Fix: check the API surface
on the OpenAP CLASS (`hasattr(openassetpricing.OpenAP, method)`) instead of an
instance — the 4 methods are class-level defs, so the scout signal (import
resolves + class exposed + methods present) stays in the default offline lane
with ZERO network dependency. The live instantiation+fetch path remains
covered by the `@network test_fetch_osap_returns_live` in the same file.

**Files** (7): `frontend/app/layout.tsx` (pre-paint script) ·
`frontend/app/globals.css` (pre-hydration width rule) ·
`frontend/components/AppShell.tsx` (`animate` flag + class sync) ·
`frontend/components/Sidebar.tsx` (`animate` prop + `data-sidebar-rail` +
gated transition) · `frontend/components/PriceHistoryChart.tsx` (ResizeObserver
re-park) · `tests/test_validation/test_ranking_history.py` +
`tests/test_ingest/test_osap.py` (flaky guards).

**Verification**: `ruff check .` clean · `pytest -m "not network"` → 1407
passed (was 1406 + 1 OSAP-flake; now deterministic) · `tsc --noEmit` clean on
edited files · `next build` → 506 routes · Playwright empirical: A.1 no-flash
(rail 72px across 62 frames, never wide) / A.2 transition gated (rest→none,
toggle→width, settle→none) / A.3 rotation instant (transition none, rail 72px) /
B crosshair re-park (ratio before=1 / transient=0 / after=1) — all PASS.

PHASE_STATUS_INFLIGHT.md side-file satisfies §Conventions "ship with every PR"
lockstep per PR #237 convention. No CLAUDE.md / AGENTS.md substance change
required — the fixes don't introduce a new invariant future authors must
remember beyond what the existing §Gotchas (`overflow-x: clip`, fluid root
font-size, chart remount-key) already frame.

---
61 changes: 61 additions & 0 deletions frontend/app/globals.css
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,67 @@ html.dark {
color-scheme: dark;
}

/* Pre-hydration sidebar width (Bug A — "the sidebar opens then shrinks back
* by itself" on refresh). The static export bakes the EXPANDED sidebar markup
* into every page, so on refresh the rail paints at full width, then React
* reads localStorage and collapses it — a visible widthward flash. The inline
* script in layout.tsx adds `.sidebar-collapsed` to <html> BEFORE paint when
* the user's stored choice was collapsed; this rule then renders the rail at
* its collapsed width immediately, so React's post-hydration collapse causes
* no width change at all. Mirrors how next-themes pre-paints the dark class.
* Scoped to md+ — below md the sidebar is an off-canvas drawer, not a rail, so
* `collapsed` is a no-op there. `overflow:hidden` clips the not-yet-collapsed
* inner labels for the sub-paint moment until React applies the collapsed
* layout. AppShell keeps the class in sync with the live collapsed state once
* hydrated, so the rule never fights React's width after the app is
* interactive (the class is removed the instant the rail is expanded). */
@media (min-width: 768px) {
html.sidebar-collapsed aside[data-sidebar-rail] {
width: 4rem;
max-width: 4rem;
overflow: hidden;
}
/* Render the FULL collapsed layout pre-hydration, not just the width — else
* the expanded inner content (wordmark "QuantRank" + nav labels + section
* headers) paints for a frame on a collapsed refresh, then React hydrates
* and hides it ("text pops up then disappears"). These rules mirror the
* `collapsed ? 'md:…'` Tailwind classes the React tree applies, keyed to the
* pre-paint `.sidebar-collapsed` class so the rail looks collapsed from the
* FIRST paint. They AGREE with React's classes post-hydration (both collapse
* when the class/prop is set, kept in sync by AppShell), so they never fight.
* Keep the data-rail values in step with the matching `collapsed ? …` classes
* in Sidebar.tsx. */
html.sidebar-collapsed [data-rail="hide"] {
display: none;
}
html.sidebar-collapsed [data-rail="show"] {
display: flex;
}
html.sidebar-collapsed [data-rail="header"] {
height: auto;
flex-direction: column;
justify-content: center;
gap: 0.375rem; /* md:gap-1.5 */
padding: 0.75rem 0.5rem; /* md:py-3 md:px-2 */
}
html.sidebar-collapsed [data-rail="navlink"] {
justify-content: center; /* md:justify-center */
padding-left: 0; /* md:px-0 */
padding-right: 0;
}
html.sidebar-collapsed [data-rail="chevron"] {
width: 100%; /* md:w-full */
height: auto; /* md:h-auto */
margin-left: 0; /* drops ml-auto */
padding-top: 0.375rem; /* md:py-1.5 */
padding-bottom: 0.375rem;
background-color: rgb(241 245 249); /* md:bg-slate-100 */
}
html.dark.sidebar-collapsed [data-rail="chevron"] {
background-color: rgb(30 41 59); /* md:dark:bg-slate-800 */
}
}

.font-mono,
code {
font-family: var(--font-mono);
Expand Down
12 changes: 12 additions & 0 deletions frontend/app/layout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,18 @@ export default function RootLayout({ children }: { children: React.ReactNode })
// the harmless attr mismatch on <html> needs to be suppressed.
<html lang="en" suppressHydrationWarning>
<body>
{/* Pre-paint the collapsed sidebar (Bug A). The static export always
bakes the EXPANDED rail into the HTML, so without this the rail
flashes wide → narrow on refresh once React reads localStorage.
Setting the class before the body paints lets globals.css render
the collapsed width immediately. Mirrors next-themes' pre-paint
dark-mode class; IIFE + try/catch, browser-only, never throws. */}
<script
dangerouslySetInnerHTML={{
__html:
"(function(){try{if(localStorage.getItem('quantrank.sidebar.collapsed')==='1'){document.documentElement.classList.add('sidebar-collapsed')}}catch(e){}})()",
}}
/>
<ThemeProvider>
<AppShell>{children}</AppShell>
</ThemeProvider>
Expand Down
41 changes: 38 additions & 3 deletions frontend/components/AppShell.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,13 @@ export function AppShell({ children }: { children: React.ReactNode }) {
const [collapsed, setCollapsed] = useState(false);
const [mobileOpen, setMobileOpen] = useState(false);
const [hydrated, setHydrated] = useState(false);
// `animate` gates the sidebar's width/transform transition. It is true ONLY
// for the ~250ms around an explicit user toggle (collapse chevron, mobile
// hamburger, backdrop). Hydration (refresh) and breakpoint/orientation
// changes (rotate) leave it false → the sidebar switches width/position
// INSTANTLY there, never animating. An animated width on those events is the
// "sidebar opens then shrinks back by itself" flash (Bug A).
const [animate, setAnimate] = useState(false);

useEffect(() => {
const stored = typeof window !== 'undefined' && window.localStorage.getItem(STORAGE_KEY);
Expand All @@ -27,8 +34,35 @@ export function AppShell({ children }: { children: React.ReactNode }) {
useEffect(() => {
if (!hydrated) return;
window.localStorage.setItem(STORAGE_KEY, collapsed ? '1' : '0');
// Keep the <html> class in lockstep with the live state so (a) the
// pre-paint script in layout.tsx reflects the latest choice on the NEXT
// refresh and (b) the pre-hydration width CSS (globals.css) never fights
// React once interactive — the class is removed the instant we expand.
document.documentElement.classList.toggle('sidebar-collapsed', collapsed);
}, [collapsed, hydrated]);

// Turn the transition back off after an explicit toggle settles, so the next
// refresh / rotation / resize switches width instantly (Bug A). Re-armed by
// collapsed/mobileOpen so a second toggle inside the window resets the timer.
useEffect(() => {
if (!animate) return;
const t = window.setTimeout(() => setAnimate(false), 250);
return () => window.clearTimeout(t);
}, [animate, collapsed, mobileOpen]);

const toggleCollapse = () => {
setAnimate(true);
setCollapsed((c) => !c);
};
const openMobile = () => {
setAnimate(true);
setMobileOpen(true);
};
const closeMobile = () => {
setAnimate(true);
setMobileOpen(false);
};

useEffect(() => {
if (mobileOpen) {
document.body.style.overflow = 'hidden';
Expand All @@ -44,17 +78,18 @@ export function AppShell({ children }: { children: React.ReactNode }) {
<div className="flex min-h-screen">
<Sidebar
collapsed={collapsed}
onToggleCollapse={() => setCollapsed((c) => !c)}
animate={animate}
onToggleCollapse={toggleCollapse}
mobileOpen={mobileOpen}
onMobileClose={() => setMobileOpen(false)}
onMobileClose={closeMobile}
/>

<div className="flex min-w-0 flex-1 flex-col">
<header className="sticky top-0 z-20 flex items-center gap-3 border-b border-slate-200 bg-white/95 px-4 py-2 backdrop-blur dark:border-slate-800 dark:bg-slate-950/95 md:px-6">
<button
type="button"
aria-label="Open navigation"
onClick={() => setMobileOpen(true)}
onClick={openMobile}
className="inline-flex h-9 w-9 items-center justify-center rounded-sm text-slate-600 transition-colors duration-150 hover:bg-slate-100 hover:text-slate-900 dark:text-slate-400 dark:hover:bg-slate-800 dark:hover:text-slate-100 md:hidden"
>
<svg width="18" height="18" viewBox="0 0 24 24" fill="none" stroke="currentColor" strokeWidth="2" strokeLinecap="round" strokeLinejoin="round">
Expand Down
66 changes: 44 additions & 22 deletions frontend/components/PriceHistoryChart.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
'use client';

import { useEffect, useMemo, useState } from 'react';
import { useEffect, useMemo, useRef, useState } from 'react';
import { useTheme } from 'next-themes';
import {
Area,
Expand Down Expand Up @@ -66,31 +66,16 @@ export function PriceHistoryChart({
// remount is how we re-assert it on those events.
const [restKey, setRestKey] = useState(0);
const [layoutKey, setLayoutKey] = useState(0);
// Chart wrapper — observed for WIDTH changes to re-park the crosshair (Bug B).
const wrapperRef = useRef<HTMLDivElement | null>(null);
const { resolvedTheme } = useTheme();

useEffect(() => setMounted(true), []);

// Re-park the tooltip at the latest date when the device rotates
// portrait↔landscape. matchMedia is browser-only; the guard keeps SSR
// clean (component is 'use client', so this never runs server-side).
// The bump is DEBOUNCED ~300ms after the orientation event so the
// remount lands AFTER ResponsiveContainer has re-measured the new
// width — remounting mid-resize makes Recharts' displayDefaultTooltip
// park on index 0 instead of the latest.
useEffect(() => {
if (typeof window === 'undefined') return;
const mq = window.matchMedia('(orientation: portrait)');
let t: ReturnType<typeof setTimeout>;
const handler = () => {
clearTimeout(t);
t = setTimeout(() => setLayoutKey((k) => k + 1), 300);
};
mq.addEventListener('change', handler);
return () => {
clearTimeout(t);
mq.removeEventListener('change', handler);
};
}, []);
// Crosshair re-park on layout change (rotation / window resize / sidebar
// expand-collapse) is handled by the ResizeObserver effect just below the
// chartData memo — a width-driven detector that subsumes the old
// orientation-only matchMedia listener. See that effect for the rationale.

useEffect(() => {
let cancelled = false;
Expand Down Expand Up @@ -153,6 +138,42 @@ export function PriceHistoryChart({
return { abs, pct, positive: abs >= 0 };
}, [chartData]);

// Re-park the crosshair at the latest date after any WIDTH change settles —
// device rotation, window resize, OR the left sidebar expanding/collapsing
// (which reflows the main-content width). A width change makes
// ResponsiveContainer re-measure the chart, but Recharts applies
// `defaultIndex` only on MOUNT — so without re-asserting it the crosshair
// drifts to a stale/left x after a re-measure (the "crosshair jumps left on
// sidebar toggle" bug). Bumping `layoutKey` remounts <AreaChart>, re-running
// displayDefaultTooltip(defaultIndex) at the latest point. DEBOUNCED ~300ms
// so the remount lands AFTER the re-measure — remounting mid-resize parks on
// index 0 (far left). The width-only delta gate ignores height-only changes
// so the chart's own crosshair rendering can't trigger a spurious remount.
// ResizeObserver is browser-only; wrapperRef is null during loading/error/
// empty, so the deps re-attach the observer once the chart wrapper mounts.
useEffect(() => {
const el = wrapperRef.current;
if (!el || typeof ResizeObserver === 'undefined') return;
let lastWidth = el.getBoundingClientRect().width;
let t: ReturnType<typeof setTimeout>;
const ro = new ResizeObserver((entries) => {
const w = entries[0].contentRect.width;
if (Math.abs(w - lastWidth) < 1) return; // height-only change → ignore
lastWidth = w;
clearTimeout(t);
t = setTimeout(() => setLayoutKey((k) => k + 1), 300);
});
ro.observe(el);
return () => {
clearTimeout(t);
ro.disconnect();
};
// Re-attach only when the wrapper's existence changes (loading/error/data);
// NOT on chartData.length — the wrapper div persists across period switches
// (only the inner <AreaChart> remounts via key), so the observer stays valid
// and a period change needn't disconnect/re-observe.
}, [loading, error, data]);

if (loading) {
// Skeleton placeholder — shimmer blocks roughly match the layout
// shipped after load (current-price headline + change indicator +
Expand Down Expand Up @@ -467,6 +488,7 @@ export function PriceHistoryChart({
then sized itself to and sustained; the document clip stops the
layout viewport from ever growing. */}
<div
ref={wrapperRef}
className="h-64 w-full [&_.recharts-surface]:overflow-visible"
style={{ touchAction: 'pan-y' }}
onPointerDown={(e) => {
Expand Down
Loading