Merge ticket 053: move sort selector from Log Preview to Timeline

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XFUNZwCvz3kcod3352ZR2o

tickets/053-move-sort-selector-to-timeline.md

@@ -1,69 +0,0 @@
-# 053 — Move the log sort selector from Log Preview to the Timeline
-
-## Summary
-
-The "Sort by" control lives in the wrong component. It's currently
-rendered inside `LogPreview`'s own section header
-(`webapp/src/components/LogPreview.tsx:327-336`) as a `SortPicker`
-(`LogPreview.tsx:68-107`, a `<select>` plus an "Fall back to ottyl_seq"
-checkbox), with its `LogSortSettings` type and
-`loadLogSortSettings`/`saveLogSortSettings` persistence helpers
-(`LogPreview.tsx:37-66`, localStorage key `ottyl.logPreview.sort`) also
-defined there. `LogTimeline.tsx` has no sort-related prop at all
-(`LogTimeline.tsx:44-56`) — its header only has time-range preset buttons
-and a "Clear selection" link (`LogTimeline.tsx:118-149`).
-
-The actual sort state is owned one level up, duplicated identically in
-`App.tsx`'s two top-level render modes: `FullApp`
-(`App.tsx:389-394`) and `StandaloneLog` (`App.tsx:263-268`), both via
-`useState<LogSortSettings>(loadLogSortSettings)` plus a
-`handleSortSettingsChange` callback. Both currently pass
-`sortSettings`/`onSortSettingsChange` only to `LogPreview`
-(`App.tsx:361`, `App.tsx:527`), never to `LogTimeline`
-(`App.tsx:237-243`, `354-360`/`520-526`). The setting already drives
-`fetchLogs` (`App.tsx:319-326`, `464-471`, dependency at `App.tsx:347`,
-`494`), which forwards `sortBy`/`sortMissingFallback` to
-`GET /api/logs` via `fetchLogs` in `webapp/src/lib/api.ts:242-262`. The
-backend already fully supports this — `src/dto.rs:86-90`'s
-`sort_by`/`sort_missing_fallback` fields, validated and applied in
-`src/engine.rs:1223-1281` (400 if `sort_by` isn't a declared param,
-`engine.rs:1227`; missing-value fallback behavior at `engine.rs:1260-1281`).
-The timeline query/DTO backing `fetchTimeline`
-(`webapp/src/lib/api.ts:264-274`) has no `sort_by` field and doesn't need
-one — this is a frontend-only relocation of where the control lives, not
-a backend change.
-
-## Scope
-
-- Move `SortPicker` (and ideally the `LogSortSettings` type plus
-  `loadLogSortSettings`/`saveLogSortSettings`) out of `LogPreview.tsx`
-  into `LogTimeline.tsx`, rendering it in the Timeline's header
-  (`LogTimeline.tsx:118-149`) alongside the existing range controls.
-- Add `sortSettings`/`onSortSettingsChange` props to `LogTimeline`.
-- Update both `App.tsx` render sites (`FullApp` around line 237,
-  `StandaloneLog` around line 354/520) to pass `sortSettings`/
-  `onSortSettingsChange` to `LogTimeline` instead of `LogPreview`.
-- `LogPreview` keeps rendering the `logs` prop as already sorted by the
-  parent's `fetchLogs` call — remove its own `SortPicker` usage and the
-  now-unused sort-control markup from its header, but keep consuming
-  `sortSettings` if it still needs read access to it (e.g. for a
-  "sorted by X" label), or drop that too if not needed.
-- No backend changes — `sort_by`/`sort_missing_fallback` on
-  `GET /api/logs` are unaffected.
-
-## Acceptance criteria
-
-- The "Sort by" selector and "fall back to ottyl_seq" checkbox appear in
-  the Timeline component's header, not in Log Preview's.
-- Changing sort order from the Timeline's control changes the order of
-  entries shown in Log Preview (same behavior as today, just relocated).
-- The chosen sort setting still persists across reloads (localStorage key
-  can be renamed if it moves file, but must not silently reset to
-  default on the first load after this change — migrate or fall back
-  gracefully).
-- `make build` (webapp `tsc -b && vite build`) passes with no leftover
-  unused imports/types in `LogPreview.tsx`.
-
-## Depends on
-
-None.

webapp/src/App.tsx

@@ -6,10 +6,13 @@ import {
   TIME_RANGE_PRESETS,
   computeBucketMs,
   computePresetRange,
+  loadLogSortSettings,
+  saveLogSortSettings,
+  type LogSortSettings,
   type TimeRange,
   type TimeRangePresetId,
 } from './components/LogTimeline'
-import { LogPreview, loadLogSortSettings, saveLogSortSettings, type LogSortSettings } from './components/LogPreview'
+import { LogPreview } from './components/LogPreview'
 import { Params } from './components/Params'
 import { Metrics } from './components/Metrics'
 import { Dashboards } from './components/Dashboards'
@@ -357,8 +360,11 @@ function StandaloneLog() {
             onSelectRange={handleManualRangeSelect}
             activePreset={activePreset}
             onSelectPreset={selectPreset}
+            paramNames={params.map((p) => p.name)}
+            sortSettings={sortSettings}
+            onSortSettingsChange={handleSortSettingsChange}
           />
-          <LogPreview logs={logs} params={params} sortSettings={sortSettings} onSortSettingsChange={handleSortSettingsChange} />
+          <LogPreview logs={logs} params={params} />
         </div>
       </div>
     </StandalonePage>
@@ -523,8 +529,11 @@ function FullApp() {
               onSelectRange={handleManualRangeSelect}
               activePreset={activePreset}
               onSelectPreset={selectPreset}
+              paramNames={params.map((p) => p.name)}
+              sortSettings={sortSettings}
+              onSortSettingsChange={handleSortSettingsChange}
             />
-            <LogPreview logs={logs} params={params} sortSettings={sortSettings} onSortSettingsChange={handleSortSettingsChange} />
+            <LogPreview logs={logs} params={params} />
           </div>
           {!logViewerCollapsed && <ResizeHandle onResize={handleLogViewerResize} onResizeEnd={handleLogViewerResizeEnd} />}
           <Params params={params} onDeclared={loadParams} />

webapp/src/components/LogPreview.tsx

@@ -25,87 +25,6 @@ function saveVisibleParamColumns(columns: string[]) {
   }
 }
 
-// Ticket 045: ordering key + missing-param handling for the log preview / timeline. Scoped like
-// `VISIBLE_PARAM_COLUMNS_STORAGE_KEY` and `App.tsx`'s `LOG_VIEWER_HEIGHT_STORAGE_KEY` above — one
-// shared "log viewer" concept, so a single localStorage key covers both the inline viewer
-// (`FullApp`) and its popout (`StandaloneLog`) rather than each keeping its own copy, which would
-// surprise a user switching between the two. There's no dashboard/panel-scoped analog for this
-// setting to divide by (unlike e.g. per-dashboard panels), so a single global key matches how the
-// rest of the log viewer's own settings are scoped.
-const LOG_SORT_STORAGE_KEY = 'ottyl.logPreview.sort'
-
-export interface LogSortSettings {
-  /** An already-declared param name to order log lines by, or `null` for the default (`ottyl_ts`-tracking) order. */
-  sortBy: string | null
-  /** Only meaningful when `sortBy` is set. `true` (default) orders lines missing that param by `ottyl_seq`, appended after the sorted lines; `false` drops them from view. */
-  sortMissingFallback: boolean
-}
-
-const DEFAULT_LOG_SORT_SETTINGS: LogSortSettings = { sortBy: null, sortMissingFallback: true }
-
-export function loadLogSortSettings(): LogSortSettings {
-  try {
-    const raw = localStorage.getItem(LOG_SORT_STORAGE_KEY)
-    if (!raw) return DEFAULT_LOG_SORT_SETTINGS
-    const parsed = JSON.parse(raw)
-    if (!parsed || typeof parsed !== 'object') return DEFAULT_LOG_SORT_SETTINGS
-    const sortBy = typeof parsed.sortBy === 'string' ? parsed.sortBy : null
-    const sortMissingFallback = typeof parsed.sortMissingFallback === 'boolean' ? parsed.sortMissingFallback : true
-    return { sortBy, sortMissingFallback }
-  } catch {
-    return DEFAULT_LOG_SORT_SETTINGS
-  }
-}
-
-export function saveLogSortSettings(settings: LogSortSettings) {
-  try {
-    localStorage.setItem(LOG_SORT_STORAGE_KEY, JSON.stringify(settings))
-  } catch {
-    // best-effort — private browsing or full storage just means the selection won't persist across reloads
-  }
-}
-
-function SortPicker({
-  paramNames,
-  settings,
-  onChange,
-}: {
-  paramNames: string[]
-  settings: LogSortSettings
-  onChange: (next: LogSortSettings) => void
-}) {
-  return (
-    <div className="flex items-center gap-2 text-xs text-text-secondary">
-      <label className="flex items-center gap-1">
-        Sort by
-        <select
-          value={settings.sortBy ?? ''}
-          onChange={(e) => onChange({ ...settings, sortBy: e.target.value || null })}
-          className="rounded border border-border-soft bg-surface-0 px-1 py-0.5 text-text"
-        >
-          <option value="">ottyl_ts (default)</option>
-          {paramNames.map((name) => (
-            <option key={name} value={name}>
-              {name}
-            </option>
-          ))}
-        </select>
-      </label>
-      {settings.sortBy && (
-        <label className="flex cursor-pointer items-center gap-1" title="How lines missing the sort param are handled">
-          <input
-            type="checkbox"
-            checked={settings.sortMissingFallback}
-            onChange={(e) => onChange({ ...settings, sortMissingFallback: e.target.checked })}
-            className="accent-text-muted"
-          />
-          Fall back to ottyl_seq
-        </label>
-      )}
-    </div>
-  )
-}
-
 function ColumnPicker({
   paramNames,
   visible,
@@ -293,14 +212,10 @@ function useScrollAnchor(containerRef: React.RefObject<HTMLDivElement | null>, d
 export function LogPreview({
   logs,
   params,
-  sortSettings,
-  onSortSettingsChange,
 }: {
   logs: LogEntry[]
+  /** Ticket 045/053: already sorted by the caller's `fetchLogs` call per its own sort setting (owned and rendered up in `LogTimeline`'s header now) — this component just renders whatever order it's given. */
   params: ParamDeclaration[]
-  /** Ticket 045: current ordering choice, owned by the caller (it also feeds `fetchLogs`) rather than internally like `visibleParamColumns` — see `App.tsx`'s poll loop. */
-  sortSettings: LogSortSettings
-  onSortSettingsChange: (next: LogSortSettings) => void
 }) {
   const paramNames = params.map((d) => d.name)
   const [visibleParamColumns, setVisibleParamColumns] = useState<string[]>(loadVisibleParamColumns)
@@ -330,7 +245,6 @@ export function LogPreview({
           <HelpIcon sectionId="log-preview" label="Help: Log preview" />
         </div>
         <div className="flex items-center gap-3">
-          <SortPicker paramNames={paramNames} settings={sortSettings} onChange={onSortSettingsChange} />
           <ColumnPicker paramNames={paramNames} visible={visible} onToggle={toggleColumn} />
         </div>
       </div>

webapp/src/components/LogTimeline.tsx

@@ -41,18 +41,116 @@ function formatTimestamp(ms: number): string {
   return new Date(ms).toLocaleTimeString()
 }
 
+// Ticket 053: moved here from LogPreview.tsx, along with the `SortPicker` control below — the
+// setting now renders in the Timeline's header, but it still drives `fetchLogs` up in App.tsx the
+// same way it always did. Renamed from the old `ottyl.logPreview.sort` key since the control no
+// longer lives in "log preview"; `loadLogSortSettings` still falls back to the old key so an
+// existing choice survives this move instead of silently resetting on first load.
+const LOG_SORT_STORAGE_KEY = 'ottyl.logTimeline.sort'
+const LEGACY_LOG_SORT_STORAGE_KEY = 'ottyl.logPreview.sort'
+
+export interface LogSortSettings {
+  /** An already-declared param name to order log lines by, or `null` for the default (`ottyl_ts`-tracking) order. */
+  sortBy: string | null
+  /** Only meaningful when `sortBy` is set. `true` (default) orders lines missing that param by `ottyl_seq`, appended after the sorted lines; `false` drops them from view. */
+  sortMissingFallback: boolean
+}
+
+const DEFAULT_LOG_SORT_SETTINGS: LogSortSettings = { sortBy: null, sortMissingFallback: true }
+
+function parseLogSortSettings(raw: string | null): LogSortSettings | null {
+  if (!raw) return null
+  try {
+    const parsed = JSON.parse(raw)
+    if (!parsed || typeof parsed !== 'object') return null
+    const sortBy = typeof parsed.sortBy === 'string' ? parsed.sortBy : null
+    const sortMissingFallback = typeof parsed.sortMissingFallback === 'boolean' ? parsed.sortMissingFallback : true
+    return { sortBy, sortMissingFallback }
+  } catch {
+    return null
+  }
+}
+
+export function loadLogSortSettings(): LogSortSettings {
+  try {
+    return (
+      parseLogSortSettings(localStorage.getItem(LOG_SORT_STORAGE_KEY)) ??
+      parseLogSortSettings(localStorage.getItem(LEGACY_LOG_SORT_STORAGE_KEY)) ??
+      DEFAULT_LOG_SORT_SETTINGS
+    )
+  } catch {
+    return DEFAULT_LOG_SORT_SETTINGS
+  }
+}
+
+export function saveLogSortSettings(settings: LogSortSettings) {
+  try {
+    localStorage.setItem(LOG_SORT_STORAGE_KEY, JSON.stringify(settings))
+  } catch {
+    // best-effort — private browsing or full storage just means the selection won't persist across reloads
+  }
+}
+
+function SortPicker({
+  paramNames,
+  settings,
+  onChange,
+}: {
+  paramNames: string[]
+  settings: LogSortSettings
+  onChange: (next: LogSortSettings) => void
+}) {
+  return (
+    <div className="flex items-center gap-2 text-xs text-text-secondary">
+      <label className="flex items-center gap-1">
+        Sort by
+        <select
+          value={settings.sortBy ?? ''}
+          onChange={(e) => onChange({ ...settings, sortBy: e.target.value || null })}
+          className="rounded border border-border-soft bg-surface-0 px-1 py-0.5 text-text"
+        >
+          <option value="">ottyl_ts (default)</option>
+          {paramNames.map((name) => (
+            <option key={name} value={name}>
+              {name}
+            </option>
+          ))}
+        </select>
+      </label>
+      {settings.sortBy && (
+        <label className="flex cursor-pointer items-center gap-1" title="How lines missing the sort param are handled">
+          <input
+            type="checkbox"
+            checked={settings.sortMissingFallback}
+            onChange={(e) => onChange({ ...settings, sortMissingFallback: e.target.checked })}
+            className="accent-text-muted"
+          />
+          Fall back to ottyl_seq
+        </label>
+      )}
+    </div>
+  )
+}
+
 export function LogTimeline({
   buckets,
   range,
   onSelectRange,
   activePreset,
   onSelectPreset,
+  paramNames,
+  sortSettings,
+  onSortSettingsChange,
 }: {
   buckets: TimelineBucket[]
   range: TimeRange | null
   onSelectRange: (range: TimeRange | null) => void
   activePreset: TimeRangePresetId | null
   onSelectPreset: (id: TimeRangePresetId) => void
+  /** Ticket 045/053: current ordering choice, owned by the caller (it also feeds `fetchLogs`). Omitted by callers with no log list to sort (e.g. the standalone metrics/dashboards popout). */
+  paramNames?: string[]
+  sortSettings?: LogSortSettings
+  onSortSettingsChange?: (next: LogSortSettings) => void
 }) {
   const containerRef = useRef<HTMLDivElement>(null)
   const [dragStartX, setDragStartX] = useState<number | null>(null)
@@ -122,7 +220,11 @@ export function LogTimeline({
           Timeline
           <HelpIcon sectionId="timeline" label="Help: Timeline" />
         </span>
-        <div className="flex items-center gap-1.5 text-xs text-text-subtle">
+        <div className="flex items-center gap-3 text-xs text-text-subtle">
+          {sortSettings && onSortSettingsChange && (
+            <SortPicker paramNames={paramNames ?? []} settings={sortSettings} onChange={onSortSettingsChange} />
+          )}
+          <div className="flex items-center gap-1.5">
           <div className="flex items-center gap-1">
             {TIME_RANGE_PRESETS.map((preset) => (
               <button
@@ -145,6 +247,7 @@ export function LogTimeline({
               Clear selection
             </button>
           )}
+          </div>
         </div>
       </div>
       <div className="p-3">