From f34dbba6c74b67c3b5c2f48d1edce9d850488d44 Mon Sep 17 00:00:00 2001 From: Hugh Kaznowski Date: Sun, 6 Sep 2026 14:37:03 +0100 Subject: [PATCH] Update skill and tickets --- SKILL.md | 15 +++ ...70-cli-functionality-needs-fleshing-out.md | 43 ++++++++ ...-bar-button-placement-and-popup-clarity.md | 49 +++++++++ tickets/072-log-preview-format-column.md | 44 ++++++++ ...stion-to-engine-processing-lag-tracking.md | 64 +++++++++++ tickets/074-timeline-sort-still-broken.md | 68 ++++++++++++ .../075-investigate-api-unresponsiveness.md | 60 ++++++++++ ...ream-whitespace-columnar-parsing-broken.md | 104 ++++++++++++++++++ 8 files changed, 447 insertions(+) create mode 100644 tickets/070-cli-functionality-needs-fleshing-out.md create mode 100644 tickets/071-filter-bar-button-placement-and-popup-clarity.md create mode 100644 tickets/072-log-preview-format-column.md create mode 100644 tickets/073-ingestion-to-engine-processing-lag-tracking.md create mode 100644 tickets/074-timeline-sort-still-broken.md create mode 100644 tickets/075-investigate-api-unresponsiveness.md create mode 100644 tickets/076-log-stream-whitespace-columnar-parsing-broken.md diff --git a/SKILL.md b/SKILL.md index b9a4090..86df36b 100644 --- a/SKILL.md +++ b/SKILL.md @@ -32,6 +32,21 @@ ottyl mcp --port 51423 This exposes six tools, each a thin proxy onto the target instance's HTTP API. +## Get the latest SKILL.md + +This file is compiled into the `ottyl` binary, so a copy you have on disk can lag behind the +version a running instance actually serves (e.g. after an upgrade). A running instance always +serves its own current copy read-only over HTTP — fetch it directly rather than trusting a stale +local file: + +``` +curl http://localhost:/SKILL.md +``` + +Use the port from `ottyl sessions` (or the `--port`/`--session` you passed to `ottyl mcp`). There +is no CLI subcommand for this and no way to update it at runtime — the served content only changes +when the binary is rebuilt with a new `SKILL.md`. + ## Tools ### `query_logs` diff --git a/tickets/070-cli-functionality-needs-fleshing-out.md b/tickets/070-cli-functionality-needs-fleshing-out.md new file mode 100644 index 0000000..b6eda89 --- /dev/null +++ b/tickets/070-cli-functionality-needs-fleshing-out.md @@ -0,0 +1,43 @@ +# 070 — CLI functionality needs to be greater — scope this out + +## Summary + +Wishlist feedback: "CLI functionality needs to be greater - todo +fleshout." This is a scoping ticket, not an implementation one — the +request as given doesn't say which CLI surface is thin (flags to +`ottyl` itself vs. profile/dashboard management vs. something +scriptable/headless) or what "greater" should mean in practice. + +Current CLI surface, for reference: `main.rs`'s arg parsing wires +stdin/`--file` tailing, `--metrics-url` polling, and profile +selection/loading (see `settings.rs`/`profile.rs`) into engine startup. +Anything beyond piping a command and viewing the dashboard in a browser +(inspecting state, managing profiles, exporting data, scripting against +a running instance) has no CLI path today — it's all through the web +UI's API calls. + +## Scope + +- Talk to Hugh about what's actually missing before writing an + implementation ticket. Candidate directions worth raising: + - A way to inspect current state (params/metrics/alert rules, log + counts) from the terminal without opening the browser. + - Profile management from the CLI (list/create/switch profiles) + rather than only through the web UI's settings. + - A non-interactive/headless mode for CI-type use (e.g. run for N + seconds against piped input, dump a summary, exit) — distinct from + today's "serve until the piped process exits" model. + - Flags to declare params/metrics/dashboards at launch without + hand-editing the profile file first. +- Once scoped, split into one or more concrete tickets with specific + acceptance criteria — this ticket should be closed by replacement, + not by a partial implementation guessing at the ask. + +## Acceptance criteria + +- N/A — this ticket's output is a conversation and a follow-up + ticket (or tickets), not code. + +## Depends on + +None. diff --git a/tickets/071-filter-bar-button-placement-and-popup-clarity.md b/tickets/071-filter-bar-button-placement-and-popup-clarity.md new file mode 100644 index 0000000..2ffdadd --- /dev/null +++ b/tickets/071-filter-bar-button-placement-and-popup-clarity.md @@ -0,0 +1,49 @@ +# 071 — Filter bar's syntax button/popup is ugly and incoherent + +## Summary + +Wishlist feedback, verbatim: "UI filter ugly syntax. The button moved +to the right, away from the existing button cluster. The popup is a +incoherent mess and should be clearer." + +Two distinct complaints bundled together: + +1. Layout regression — some button (the filter-help/syntax button, + presumably introduced or moved by ticket 065's filter-bar help + rewrite) now sits away from the rest of the filter bar's button + cluster in `FilterBar.tsx` (`webapp/src/components/FilterBar.tsx`, + currently 67 lines), instead of grouped with the others. +2. Content/clarity — whatever popup that button opens (the filter + syntax help introduced/reworked in ticket 065, + "filter-bar-help-should-describe-filter-syntax-not-regex") reads as + an incoherent mess rather than a clear reference. + +## Scope + +- Reproduce in the running app: open the filter bar, locate the + syntax/help button, and confirm which button has drifted from the + cluster and what "incoherent" means concretely (wall of text? + disorganized sections? unclear examples?) — the wishlist entry + doesn't specify, so this needs eyes-on before a fix. +- Fix the button's position so it's grouped with the filter bar's other + controls again. +- Rework the popup's content/layout for clarity — likely restructuring + into clearly separated sections (e.g. by filter feature: regex match, + metric comparison, datetime, sort) with short examples per section, + rather than a single undifferentiated block. +- Out of scope: changing the underlying filter grammar itself + (`filter_parser.pest`/`filter_parser.rs`) — this is presentation only. + +## Acceptance criteria + +- The filter syntax button sits back with the filter bar's other + buttons, not isolated to one side. +- The help popup reads as a structured, scannable reference rather than + a single dense block — verified visually in the running app (`make + dev`), not just by inspecting JSX. + +## Depends on + +None. Touches the same area as ticket 065 and ticket 066 +("filter-bar-value-doesnt-reflect-active-preset-or-sort") — check those +haven't already reshuffled this layout before starting. diff --git a/tickets/072-log-preview-format-column.md b/tickets/072-log-preview-format-column.md new file mode 100644 index 0000000..033cb06 --- /dev/null +++ b/tickets/072-log-preview-format-column.md @@ -0,0 +1,44 @@ +# 072 — Log Preview needs a format column (`ottyl_fmt`) + +## Summary + +Wishlist feedback: "Log preview needs format column." `ottyl_fmt` +(json/logfmt/none, `crate::logfmt::detect_format`) already exists as a +reserved, filterable pseudo-param (`OTTYL_RESERVED_PARAM_NAMES`, +`src/engine.rs:1960`; filter-bar support via +`compile_ottyl_fmt_comparison`, `src/engine.rs:1067`) — you can already +filter `ottyl_fmt==json` in the search bar. But it isn't surfaced as a +visible column in the Log Preview table +(`webapp/src/components/LogPreview.tsx`), and it can't be, because +`LogEntryDto` (`src/dto.rs:10-16`) doesn't include it: `params` only +holds declared `ParamDeclaration` matches, and format detection is +computed line-locally elsewhere, never attached to the returned log +entry. + +## Scope + +- Add the per-line detected format to `LogEntryDto` (`src/dto.rs:10`), + populated wherever log entries are assembled for the `Logs` query + response in `engine.rs` (reuse `crate::logfmt::detect_format`, the + same detection `ottyl_fmt` filtering already calls). +- Add `ottyl_fmt` as a column option in Log Preview's `ColumnPicker` + (`webapp/src/components/LogPreview.tsx:28-56`) alongside declared + params — it's a reserved pseudo-param like `ottyl_seq`/`ottyl_ts`, so + treat it the way the params list already flags `reserved` + (`dto.rs:38`, `ParamDeclarationDto.reserved`) rather than requiring + it be "declared" to show up as a pickable column. +- Render the column value as the short format label (`json`/`logfmt`/ + `none`) matching what `ottyl_fmt` filter comparisons already produce. + +## Acceptance criteria + +- A "Format" (or `ottyl_fmt`) column is available in the Log Preview's + column picker and, once toggled on, shows the correct detected format + per row — json lines show `json`, logfmt lines show `logfmt`, neither + shows `none`. +- No change to filter-bar `ottyl_fmt` comparison behavior (still works + as before). + +## Depends on + +None. diff --git a/tickets/073-ingestion-to-engine-processing-lag-tracking.md b/tickets/073-ingestion-to-engine-processing-lag-tracking.md new file mode 100644 index 0000000..ee133f8 --- /dev/null +++ b/tickets/073-ingestion-to-engine-processing-lag-tracking.md @@ -0,0 +1,64 @@ +# 073 — Track and surface ingestion-to-engine-processing lag (high watermark) + +## Summary + +Wishlist feedback, verbatim: + +> There needs to be a ingstion to parsing latency on frontend. +> Basically when a log is read, it gets what would be the `ottyl_ts` +> value. However, the engine can lag on ingestion. That lag needs to +> be tracked as a high watermark. We can have a collection that stores +> say 5 highest values within the last 5 seconds. If a value in that +> collection is older than the 5s window, it gets replaced. If a value +> is higher than the minimum current, it gets replaced. The frontend +> will display the highest single value (humantime duration lag, or ms +> lag) alongside where it displays the logs and allocation size. + +Confirmed the mechanism this is about: `ingest.rs` stamps +`timestamp: Timestamp::now()` at read time (this becomes `ottyl_ts`, +e.g. `ingest.rs:25`/`:107`) *before* sending `EngineMessage::Ingest` +over the mpsc channel to the engine actor task. The engine only +processes it whenever the actor's message loop gets scheduled to it — +under load (large backlog, slow query in flight, etc.) that gap can +grow. Today nothing measures `(engine processing time) - (ottyl_ts)`, +so there's no visibility into ingest backpressure. + +The "allocation size" reference point is `approx_bytes_total` +(`src/dto.rs:80`, ticket 011), currently shown in +`webapp/src/components/NavMenu.tsx:104` in the stats popover — that's +the existing display location the lag figure should sit alongside. + +## Scope + +- In the engine actor's ingest handling (where `EngineMessage::Ingest` + is received, `engine.rs`), compute `lag = Timestamp::now() - + message.timestamp` at the moment of processing. +- Maintain a small rolling high-watermark collection per the spec + above: keep up to 5 entries, each valid only within the last 5 + seconds; a new lag value replaces the current minimum-held entry if + it's larger, and any entry older than the 5s window is evicted + (replaced) regardless of magnitude. Expose the current max of that + collection. +- Add this figure to whatever stats DTO backs the popover + `approx_bytes_total` comes from, and to the corresponding stats + query/response path in `api.rs`. +- Display it in `NavMenu.tsx` next to `approx_bytes_total` (`:104`), + formatted as a humantime-style duration for larger lag and plain ms + for small values — match whatever precedent this codebase already + uses for duration formatting (check `webapp/src/lib` for an existing + humanize helper before writing a new one). + +## Acceptance criteria + +- Under normal (non-lagged) ingestion, the displayed value stays near + zero. +- Artificially delaying engine processing (e.g. a slow query + saturating the actor's message loop, or a synthetic test) causes the + displayed high-watermark to rise and correctly decays out of the + rolling 5s window afterward. +- The value is visible in the same UI location as + `approx_bytes_total`. + +## Depends on + +None. diff --git a/tickets/074-timeline-sort-still-broken.md b/tickets/074-timeline-sort-still-broken.md new file mode 100644 index 0000000..dfa1082 --- /dev/null +++ b/tickets/074-timeline-sort-still-broken.md @@ -0,0 +1,68 @@ +# 074 — Timeline sort-by-property still doesn't work after tickets 059-061 + +## Summary + +Wishlist feedback, verbatim: "The sorting on the timeline still does +not work. I have tried sorting things that I know are sorted (logs +param timestamp) and things that arent (thread name or pid). It does +not work (change view, index etc). If it's taking time to index, that +needs to be visible." + +This is a regression/bug report against functionality that should +already exist: sort-by-param was added in ticket 045, and tickets +060/061 (resolved in commit `de9ba6f`, "Resolve tickets 059-061: +generic log-stream columns + sort helper indexes") replaced the naive +full-materialize-then-sort with an incremental helper-index scheme +(`SortHelperIndexEntry`, `build_sort_helper_index`, +`update_sort_helper_indexes`, `purge_stale_sort_helper_indexes` in +`engine.rs:1332-1431`), wired into the `EngineQuery::Logs` sorted +branch around `engine.rs:1598-1634`. On the frontend, `sortBy`/ +`sortMissingFallback` are threaded from `LogTimeline.tsx`'s settings +through `App.tsx` (`:349-350`, `:505-506`) into `fetchLogs` +(`webapp/src/lib/api.ts:256-266`). Nothing found in a static read of +this wiring looks obviously broken, so this needs live reproduction, +not just a code read — treat the below as an investigation, not an +assumed root cause. + +## Scope + +- Reproduce with `make dev` against real piped input: declare a param + covering thread name or PID, set the timeline's sort dropdown to it, + and confirm whether results actually reorder. Also test sorting by an + already-monotonic value (`ottyl_ts` or a declared timestamp param) to + isolate whether the bug is "sort does nothing" vs. "sort scrambles an + already-correct order." +- Check whether the helper-index rebuild triggers correctly on first + use per sort key (`engine.rs:1604`, `.entry(sort_by...).or_insert_with`) + — a stale/empty helper index reused across a param rename or + re-declaration (see the ticket-061-era comments at `engine.rs:225-233` + about helper indexes needing invalidation when a declaration changes) + is one plausible cause worth ruling in or out. +- Check whether the frontend is actually sending `sort_by` on every + poll/refresh, not just the initial query — a poll path that omits it + (`webapp/src/lib/polling.ts`) would look exactly like "sorting + doesn't work" despite correct backend logic. +- Add visibility for the "still indexing" case the wishlist explicitly + calls out: if a helper index is being built (or rebuilt) for a + newly-selected sort key and that's slow enough to be user-visible, + surface a loading/building state in the timeline UI rather than + silently returning unsorted or partial results while it builds. +- Write a regression test once the actual defect is identified — + ticket 060's existing tests apparently didn't catch this, so identify + what scenario they miss. + +## Acceptance criteria + +- Sorting the timeline by a declared param (including thread + name/PID-style non-monotonic params) actually reorders displayed log + entries, verified live in the browser. +- Sorting by a param that's already effectively time-ordered doesn't + regress (still displays in the expected order). +- If a helper index build takes long enough to matter, the UI shows + that it's building rather than looking broken or stalled. + +## Depends on + +Builds on tickets 045, 060, 061 — read those and their resolving +commits (`de9ba6f` and its predecessor implementing 045) before +forming a hypothesis. diff --git a/tickets/075-investigate-api-unresponsiveness.md b/tickets/075-investigate-api-unresponsiveness.md new file mode 100644 index 0000000..c1594b0 --- /dev/null +++ b/tickets/075-investigate-api-unresponsiveness.md @@ -0,0 +1,60 @@ +# 075 — Investigate intermittent API unresponsiveness + +## Summary + +Wishlist feedback, verbatim: "The API is really unresponsive +sometimes. Create a ticket for me to investigate, not an agent. +Populate with some ideas." + +This ticket is explicitly for Hugh to investigate by hand, not for an +agent to pick up and fix — it's a starting list of hypotheses, not a +scoped implementation task. + +## Candidate causes to check + +- **Single actor task, single mpsc channel.** All engine state lives + behind one tokio task (`EngineHandle`/`EngineMessage`, + `engine.rs`), and every HTTP request in `api.rs` becomes a message + send + oneshot await. A single slow query (e.g. a large unsorted + timeline materialize, or the pre-060 sort path if ticket 074 turns + out to still be hitting a full-scan fallback) blocks the entire actor + loop — every other in-flight request queues behind it, including + cheap ones like stats or param-list lookups. Worth instrumenting: + time each `EngineMessage` variant's handling and log/expose the + slowest ones. +- **Sort helper index builds are synchronous full scans.** + `build_sort_helper_index` (`engine.rs:1383`) does a one-time full + scan of every currently-ingested log the first time a param is + sorted by, inline in the actor's message handling — with a large log + count this could itself be the stall, and would look exactly like + "API goes unresponsive briefly" right after picking a new sort + column. Related to ticket 074. +- **Prometheus/OpenMetrics polling (`ingest.rs::run_metrics_poll`)** + runs concurrently with log ingestion feeding the same channel — check + whether a slow or hanging `--metrics-url` endpoint can back up the + shared `EngineMessage` channel and starve log-ingestion/API-serving + throughput. +- **Unbounded per-request work scaling with log volume.** Any handler + that iterates the full retained log index (dashboards/PromQL + evaluation, `promql.rs`; alert rule evaluation, `alerts.rs`) rather + than an indexed range could scale badly as `max logs in memory` + grows, especially with multiple dashboard panels polling + concurrently. +- **Frontend polling cadence.** `webapp/src/lib/polling.ts` — check + whether multiple open tabs/panels each poll independently at a short + interval, potentially self-inflicting load rather than the backend + being intrinsically slow. +- Worth capturing wall-clock timing (both server-side, and via browser + devtools network timing) the next time it's reproduced, to know + whether the stall is queueing (actor busy), compute (one query is + slow), or genuinely I/O (waiting on `--metrics-url` or file tailing). + +## Scope + +Investigation only — no code changes implied by this ticket. Once a +concrete cause is found, split into a targeted fix ticket. + +## Depends on + +None, but related to ticket 074 (sort helper index scans) and touches +the same actor architecture ticket 060/061 modified. diff --git a/tickets/076-log-stream-whitespace-columnar-parsing-broken.md b/tickets/076-log-stream-whitespace-columnar-parsing-broken.md new file mode 100644 index 0000000..a47dda7 --- /dev/null +++ b/tickets/076-log-stream-whitespace-columnar-parsing-broken.md @@ -0,0 +1,104 @@ +# 076 — `log stream` declared columns get contaminated with garbage on non-columnar lines + +## Summary + +Wishlist feedback, verbatim: "The `log stream` type whitespace-separated +formatting is not working at all. I have a session `suited-gyrfalcon` +running, please investigate." + +Investigated directly against the live `suited-gyrfalcon` instance +(`ottyl sessions` → port 62059 → `/api/params`, `/api/logs`, +`/api/stats`; 236,069 ingested log lines, real `log stream` output). +Root cause confirmed — this is not a gap in header/column detection +(ticket 059's detector itself works correctly on genuine `log stream` +rows), it's that the *declared params* it produces get applied far too +broadly once they exist. + +**What's actually happening:** `auto_declare_log_stream_params` +(`engine.rs:472-497`) correctly gates *declaration* on +`is_log_stream_line(text)` (`:484`) — so it only fires column +declarations the first time it sees a genuine columnar data row. But +the resulting `ParamDeclaration.message_match_regex` for each column is +a pure **fixed character-offset** pattern with no content anchor at all +— e.g. observed live: `Activity` → `^.{55}(?P\S(?:.{0,19}\S)?)`, +`PID` → `^.{76}(?P\S(?:.{0,5}\S)?)`, `Thread` → +`^.{32}(?P\S(?:.{0,9}\S)?)` (from `log_stream_column_pattern`, +`logfmt.rs:287-303`). Once declared, these are ordinary +`ParamDeclaration`s: the *general* per-line param-derivation path +(`auto_declare_params`/`extract_param_raw`) applies every declared +param's regex to **every** ingested line, with no re-check that the +line is actually log-stream-shaped — there's no gate equivalent to +`is_log_stream_line` at derivation time, only at first-declaration time. + +Confirmed impact from the live instance: +- Sampling ~1000-line windows at several points across the 236k-line + stream, roughly **1-2% of lines get a garbage `Timestamp` value** + that isn't a real timestamp — e.g. line 306: + `"so_gencnt: 4042707 so_state: 0x0"` (a continuation line from a + wrapped multi-line network-stats dump, no relation to the columnar + format at all) got `Timestamp: "so_gencnt: 4042707 so_state: 0x0"`, + `Activity`/`PID`/etc sliced from arbitrary character offsets of that + unrelated text. Same for bracket/brace-only continuation lines + (`"}"`, `"]"`) inside pretty-printed dumps. +- Even the header line itself (sequence 0, + `"Timestamp Thread Type Activity PID TTL"`) ends up with + `Activity: "Activity"`, `Thread: "Thread"`, `Timestamp: "Timestamp"`, + `Type: "Type"` as its own param values — because ticket 052's + retroactive re-derivation (`engine.rs:540`, re-running + `auto_declare_params` over every retained line whenever a new param + is declared) re-applies the newly-declared fixed-offset regexes back + over already-ingested lines, including the header line, with no + exemption for it. +- This directly undermines ticket 074's "sorting is broken" complaint + too: sorting by `Timestamp` mixes ~1-2% garbage string values in among + otherwise lexically-sortable ISO timestamps, which would visibly + scramble ordering — worth re-checking ticket 074 once this is fixed, + the two may partially be the same underlying issue. + +This explains "not working at all" better than a full parse failure +would: the common case (genuine columnar rows) parses fine — it's that +the contamination on the ~1-2% of non-columnar lines interleaved +throughout every real `log stream` capture (multi-line dumps are +common) makes every declared log-stream column look untrustworthy. + +## Scope + +- Make the per-column `message_match_regex` self-verifying, not purely + positional: either (a) prefix each generated column/message pattern + (`log_stream_column_pattern`/`log_stream_message_pattern`, + `logfmt.rs:287-320`) with the same shape check `is_log_stream_line` + uses so the regex only matches on a genuinely columnar line, or (b) + gate the general param-derivation path itself so a param declared via + `auto_declare_log_stream_params` is only evaluated against lines + where `is_log_stream_line` holds — (a) is likely simpler since it + keeps the gate local to the declaration rather than adding a + special case to the generic derivation path. +- Fix ticket 052's retroactive re-derivation (`engine.rs:540`) to + respect that same gate, so the header line (and any other + non-columnar retained line) doesn't retroactively pick up columnar + param values. +- Add a regression test using the real contaminated lines captured + above (continuation lines like `"so_gencnt: ... so_state: ..."` and + the header line itself) alongside the existing + `log_stream_sample.txt` fixture — the current fixture and its tests + don't include a multi-line-dump continuation line, which is why + ticket 059's tests passed despite this. + +## Acceptance criteria + +- Ingesting a `log stream` capture that includes multi-line + continuation content (network stat dumps, bracketed/braced + pretty-printed blocks) does not populate `Timestamp`/`Thread`/`Type`/ + `Activity`/`PID`/`TTL` params on those continuation lines. +- The header line itself never ends up with its own column names as + values. +- Existing correctly-parsed columnar rows are unaffected. +- A regression test using real captured contamination examples (not + just the original fixture) passes. + +## Depends on + +Builds on ticket 059 (commit `de9ba6f`) and ticket 052's retroactive +re-derivation. Related to ticket 074 (timeline sort) — re-verify that +complaint after this fix lands, since garbage `Timestamp` values are a +plausible contributor to it.