Tickets for FE and actor split
This commit is contained in:
Hugh Kaznowski
@@ -5,7 +5,7 @@
|
||||
<!-- REPOWISE_AGENTS:START — Do not edit below this line. Auto-generated by Repowise. -->
|
||||
## Codebase Intelligence for ottyl (Repowise)
|
||||
|
||||
Indexed by [Repowise](https://repowise.dev). Last indexed: 2026-09-07 (commit c7173cf). Confidence: 100%.
|
||||
Indexed by [Repowise](https://repowise.dev). Last indexed: 2026-09-07 (commit 920629d). Confidence: 100%.
|
||||
### How to work in this repo
|
||||
|
||||
- **Trust the index.** `verified: true` and `_meta.complete` mean the bytes were checked against the live tree, so never re-read them. Re-read only what `bounds: "approximate"` or `_meta.stale_warning` names. `confidence` rates the prose, not the evidence: on `low` read the `fallback_targets` or `best_guesses` the reply names, and run `repowise update` and ask again if `_meta.hint` says the index is behind HEAD. `index_behind: true` alone is informational.
|
||||
@@ -64,13 +64,13 @@ Covers the 3 source files in 2 directories under…
|
||||
|
||||
### Files that need care (bug-fix history first, then churn — check `get_risk` before editing)
|
||||
- `webapp/src/App.tsx` — 3 bug fixes, last fix today; 22 commits/90d
|
||||
- `webapp/src/components/Dashboards.tsx` — 3 bug fixes, last fix today; 17 commits/90d
|
||||
- `webapp/src/components/Params.tsx` — 2 bug fixes, last fix today; 15 commits/90d
|
||||
- `webapp/src/components/Dashboards.tsx` — 3 bug fixes, last fix yesterday; 17 commits/90d
|
||||
- `webapp/src/components/Params.tsx` — 2 bug fixes, last fix yesterday; 15 commits/90d
|
||||
- `webapp/src/components/NavMenu.tsx` — 1 bug fix, last fix today; 22 commits/90d
|
||||
- `webapp/src/components/Docs.tsx` — 1 bug fix, last fix today; 17 commits/90d
|
||||
|
||||
### Code health
|
||||
Three co-equal signals: defect risk 5.95/10 avg, hotspot health 4.94/10 (declining), worst `src/engine/snapshot/mod.rs` at 2.0/10 · maintainability 7.48/10 · performance risk 16 open static I/O-in-loop / N+1 findings. Detail: `get_health()`.
|
||||
Three co-equal signals: defect risk 5.83/10 avg, hotspot health 4.96/10 (stable), worst `src/engine/snapshot/mod.rs` at 2.0/10 · maintainability 7.4/10 · performance risk 16 open static I/O-in-loop / N+1 findings. Detail: `get_health()`.
|
||||
|
||||
Critical files:
|
||||
- `webapp/src/components/LogPreview.tsx` — untested hotspot — impact −2.0
|
||||
|
||||
@@ -27,7 +27,7 @@ Backend: Rust (axum + tokio), single binary, in `src/`. Frontend: React 19 + Typ
|
||||
|
||||
**Data flow:** `main.rs` wires everything together at startup — loads `Settings`, spawns the engine actor task, replays the active profile's param/metric declarations into it (before ingest starts, since there's no way to reprocess earlier lines later), then starts ingest (stdin/file tailing and/or Prometheus polling) and the axum HTTP server.
|
||||
|
||||
- **`engine.rs`** (largest file, ~3400 lines) — the actor owning all live state: ingested log lines, declared params/metrics, computed metric buckets, alert evaluation. Runs as a single tokio task; all access is through `EngineHandle` sending `EngineMessage` over an mpsc channel and awaiting a response via oneshot — there is no shared mutable engine state accessed directly from request handlers. This is the file to read first to understand how declarations, filtering, and bucketing actually work.
|
||||
- **`engine/`** — owns all live state, split across two actor tasks under `engine/actor/`: `actor/primary.rs` (ingested log lines, param declarations) and `actor/profile.rs` (metric declarations, computed metric buckets, alert evaluation — moved off the primary per `ideas/0001-actor-split.md`). Both are plain `tokio::spawn`ed loops; `actor/mod.rs` holds the `left_right`-based `IndexReader`/`IndexReadHandle` read plumbing they share. All access is through `EngineHandle` (defined in `actor/primary.rs`, re-exported from `engine/mod.rs`) sending `EngineMessage` over an mpsc channel and awaiting a response via oneshot — there is no shared mutable engine state accessed directly from request handlers. `engine/snapshot/` is the state (`EngineSnapshot`) both actors mutate; this is the place to start reading to understand how declarations, filtering, and bucketing actually work.
|
||||
- **`api.rs`** — axum routes and HTTP-facing `ApiState` (holds `EngineHandle` + `SettingsHandle`). Translates HTTP requests into `EngineMessage`s and DTOs back into JSON. Also serves the embedded webapp.
|
||||
- **`settings.rs` / `profile.rs`** — persisted configuration. A **profile** is per-app: parsing rules (param/metric declarations) and dashboards, saved so they're there again next time that app runs through ottyl (see README's "Profiles"). Declarations are persisted here so the engine can replay them on restart; `api.rs` handlers that mutate engine state (e.g. `declare_metric`) call a corresponding persist method here so the two stay in sync — this dual-write is a recurring pattern (see `ApiState::persist_param_declaration` / `persist_metric_declaration` in `api.rs`), and adding a new declaration-mutating endpoint means updating both sides plus the load-time replay path.
|
||||
- **`filter_parser.rs`** (grammar in `filter_parser.pest`, via `pest`) + **`filter_datetime.rs`** — parse the search/filter bar syntax (regex matches, metric comparisons like `latency_ms[value]<200`, relative/absolute datetimes).
|
||||
|
||||
@@ -0,0 +1,46 @@
|
||||
//! The two actor tasks that own all live engine state, plus the `left_right` read plumbing they
|
||||
//! share.
|
||||
//!
|
||||
//! [`primary`] owns ingested log lines and param declarations; [`profile`] owns metric
|
||||
//! declarations, bucket data, and alert evaluation (moved off the primary per tickets 103-105,
|
||||
//! see `profile`'s module doc). Each is a `tokio::spawn`ed loop reading from its own `mpsc`
|
||||
//! channel -- there is no shared mutable state accessed directly by callers, only the handles
|
||||
//! and read-only [`IndexReader`]s below.
|
||||
|
||||
use crate::engine::snapshot::EngineSnapshot;
|
||||
|
||||
pub(crate) mod primary;
|
||||
pub(crate) mod profile;
|
||||
|
||||
/// A read-only view over one actor's [`EngineSnapshot`], obtained via [`IndexReader::handle`].
|
||||
/// Cheap to acquire per-query; the actual zero-copy read guard comes from `enter()`.
|
||||
pub struct IndexReadHandle(left_right::ReadHandle<EngineSnapshot>);
|
||||
|
||||
impl IndexReadHandle {
|
||||
pub(crate) fn enter(&self) -> Option<left_right::ReadGuard<'_, EngineSnapshot>> {
|
||||
self.0.enter()
|
||||
}
|
||||
}
|
||||
|
||||
/// A cloneable factory for [`IndexReadHandle`]s over one actor's `EngineSnapshot` -- every
|
||||
/// actor (primary and profile) publishes through its own `left_right::WriteHandle` but hands
|
||||
/// out reads through one of these. Not generic over the `left_right::Absorb` operation type,
|
||||
/// since `left_right::ReadHandle` itself isn't -- only the write side needs to know how a
|
||||
/// snapshot gets mutated.
|
||||
pub struct IndexReader {
|
||||
pub(crate) factory: left_right::ReadHandleFactory<EngineSnapshot>,
|
||||
}
|
||||
|
||||
impl Clone for IndexReader {
|
||||
fn clone(&self) -> Self {
|
||||
IndexReader {
|
||||
factory: self.factory.clone(),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl IndexReader {
|
||||
pub(crate) fn handle(&self) -> IndexReadHandle {
|
||||
IndexReadHandle(self.factory.handle())
|
||||
}
|
||||
}
|
||||
@@ -1,5 +1,5 @@
|
||||
//! The left-right write/read plumbing around `EngineSnapshot`, the actor loop that owns it,
|
||||
//! and the `EngineHandle` other tasks use to talk to that actor.
|
||||
//! The primary actor: owns ingested log lines and param declarations, and the `EngineHandle`
|
||||
//! other tasks use to talk to it (and, by proxy, to `super::profile`).
|
||||
//!
|
||||
//! Ticket 106 (giving API handlers a route to both the primary and profile actors) resolved to
|
||||
//! option (b) from its own scope note: one combined `EngineHandle`, not two separate handle
|
||||
@@ -12,6 +12,7 @@
|
||||
//! changes at all to correctly split their remove-then-redeclare sequence across both actors --
|
||||
//! they already just send `EngineMessage`s the same way they did before the split.
|
||||
|
||||
use super::IndexReader;
|
||||
use crate::config::SettingsHandle;
|
||||
use crate::consts::{INGEST_LAG_WATERMARK_MAX_ENTRIES, INGEST_LAG_WATERMARK_WINDOW_MS};
|
||||
use crate::engine::index_key::IndexKey;
|
||||
@@ -62,32 +63,6 @@ impl IndexWriter {
|
||||
}
|
||||
}
|
||||
|
||||
pub struct IndexReadHandle(left_right::ReadHandle<EngineSnapshot>);
|
||||
|
||||
impl IndexReadHandle {
|
||||
pub(crate) fn enter(&self) -> Option<left_right::ReadGuard<'_, EngineSnapshot>> {
|
||||
self.0.enter()
|
||||
}
|
||||
}
|
||||
|
||||
pub struct IndexReader {
|
||||
pub(crate) factory: left_right::ReadHandleFactory<EngineSnapshot>,
|
||||
}
|
||||
|
||||
impl Clone for IndexReader {
|
||||
fn clone(&self) -> Self {
|
||||
IndexReader {
|
||||
factory: self.factory.clone(),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl IndexReader {
|
||||
pub(crate) fn handle(&self) -> IndexReadHandle {
|
||||
IndexReadHandle(self.factory.handle())
|
||||
}
|
||||
}
|
||||
|
||||
pub struct EngineHandle {
|
||||
pub tx: mpsc::Sender<EngineMessage>,
|
||||
pub reader: IndexReader,
|
||||
@@ -174,7 +149,7 @@ fn new_channel_and_handle(
|
||||
|
||||
pub fn spawn(settings: SettingsHandle, channel_capacity: usize) -> EngineHandle {
|
||||
let (tx, rx, reader, write, version_rx) = new_channel_and_handle(channel_capacity);
|
||||
let profile = crate::engine::profile::spawn(reader.clone(), version_rx, settings);
|
||||
let profile = super::profile::spawn(reader.clone(), version_rx, settings);
|
||||
let handle = EngineHandle {
|
||||
tx,
|
||||
reader,
|
||||
@@ -196,16 +171,19 @@ pub(crate) fn test_handle(
|
||||
let handle = EngineHandle {
|
||||
tx,
|
||||
reader,
|
||||
profile_reader: crate::engine::profile::inert_reader(),
|
||||
profile_reader: super::profile::inert_reader(),
|
||||
profile_processed_sequence: processed_rx,
|
||||
};
|
||||
(handle, rx)
|
||||
}
|
||||
|
||||
/// The primary actor's loop: extracts declared + auto params only from each ingested line (per
|
||||
/// `ideas/0001-actor-split.md`'s resolved decision) -- no metrics, no alert evaluation, both of
|
||||
/// which live on `super::profile` and read this actor's publishes via `IndexReader`/`watch`.
|
||||
async fn run(
|
||||
mut rx: mpsc::Receiver<EngineMessage>,
|
||||
mut write: IndexWriter,
|
||||
profile: crate::engine::profile::ProfileHandle,
|
||||
profile: crate::engine::actor::profile::ProfileHandle,
|
||||
) {
|
||||
let mut max_logs: Option<usize> = None;
|
||||
let mut metric_sample_sequence: u64 = 0;
|
||||
@@ -340,7 +318,7 @@ async fn run(
|
||||
// proxy the message rather than mutating the primary's snapshot.
|
||||
let _ = profile
|
||||
.tx
|
||||
.send(crate::engine::profile::ProfileMessage::DeclareMetric {
|
||||
.send(crate::engine::actor::profile::ProfileMessage::DeclareMetric {
|
||||
request,
|
||||
respond_to,
|
||||
})
|
||||
@@ -353,7 +331,7 @@ async fn run(
|
||||
} => {
|
||||
let _ = profile
|
||||
.tx
|
||||
.send(crate::engine::profile::ProfileMessage::UpdateMetric {
|
||||
.send(crate::engine::actor::profile::ProfileMessage::UpdateMetric {
|
||||
name,
|
||||
request,
|
||||
respond_to,
|
||||
@@ -364,7 +342,7 @@ async fn run(
|
||||
let _ = profile
|
||||
.tx
|
||||
.send(
|
||||
crate::engine::profile::ProfileMessage::RemoveMetricDeclaration {
|
||||
crate::engine::actor::profile::ProfileMessage::RemoveMetricDeclaration {
|
||||
name,
|
||||
respond_to,
|
||||
},
|
||||
@@ -10,13 +10,13 @@
|
||||
//! own, reading it through `primary_reader` (a clone of the primary's `IndexReader`) instead.
|
||||
//! `EngineHandle`'s public API is unchanged by this ticket — `DeclareMetric`/`UpdateMetric`/
|
||||
//! `RemoveMetricDeclaration` are still sent to the primary's `mpsc` channel and proxied here by
|
||||
//! `handle.rs`'s `run()` loop; ticket 106 gives API handlers a direct route.
|
||||
//! `super::primary`'s `run()` loop; ticket 106 gives API handlers a direct route.
|
||||
|
||||
use super::IndexReader;
|
||||
use crate::config::SettingsHandle;
|
||||
use crate::engine::DeclareMetricRequest;
|
||||
use crate::engine::alerts::{self, AlertRuntimeState};
|
||||
use crate::engine::data_value::{DataValue, ParamDeclaration};
|
||||
use crate::engine::handle::IndexReader;
|
||||
use crate::engine::index_key::IndexKey;
|
||||
use crate::engine::snapshot::EngineSnapshot;
|
||||
use crate::engine::timestamp::Timestamp;
|
||||
@@ -60,7 +60,7 @@ pub(crate) struct ProfileHandle {
|
||||
pub(crate) tx: mpsc::Sender<ProfileMessage>,
|
||||
/// Read-only view of this actor's own snapshot (metric declarations, bucket data) --
|
||||
/// `MetricDeclarations`/`BucketValues`/`Promql` queries read through this (see
|
||||
/// `EngineHandle::profile_reader` in `handle.rs`) instead of the primary's reader.
|
||||
/// `EngineHandle::profile_reader` in `super::primary`) instead of the primary's reader.
|
||||
pub(crate) reader: IndexReader,
|
||||
/// The highest primary log sequence this actor has finished processing (metrics computed,
|
||||
/// alerts evaluated), or `None` before it's processed anything -- ticket 108's deterministic
|
||||
@@ -69,7 +69,7 @@ pub(crate) struct ProfileHandle {
|
||||
pub(crate) processed_sequence: watch::Receiver<Option<u64>>,
|
||||
}
|
||||
|
||||
/// A read-only view over a fresh, never-written `EngineSnapshot` -- used by `handle::test_handle`,
|
||||
/// A read-only view over a fresh, never-written `EngineSnapshot` -- used by `primary::test_handle`,
|
||||
/// which spawns no actor loop at all (so nothing would ever publish to a real profile reader
|
||||
/// anyway), just to give `EngineHandle::profile_reader` a valid value.
|
||||
#[cfg(test)]
|
||||
+3
-4
@@ -1,14 +1,13 @@
|
||||
pub mod alerts;
|
||||
pub(crate) mod actor;
|
||||
pub mod data_value;
|
||||
pub mod filter_datetime;
|
||||
pub mod filter_parser;
|
||||
pub mod handle;
|
||||
pub mod index_key;
|
||||
pub mod ingest;
|
||||
pub mod logfmt;
|
||||
pub mod message;
|
||||
pub mod metric_primitive;
|
||||
pub(crate) mod profile;
|
||||
pub mod promql;
|
||||
pub mod rate;
|
||||
pub mod snapshot;
|
||||
@@ -18,8 +17,8 @@ pub mod timestamp;
|
||||
mod tests;
|
||||
|
||||
#[cfg(test)]
|
||||
pub(crate) use handle::test_handle;
|
||||
pub use handle::{EngineHandle, spawn};
|
||||
pub(crate) use actor::primary::test_handle;
|
||||
pub use actor::primary::{EngineHandle, spawn};
|
||||
pub use message::{EngineMessage, EngineQuery, EngineResponse};
|
||||
pub(crate) use snapshot::{EngineSnapshot, eval_json_comparison};
|
||||
|
||||
|
||||
+1
-1
@@ -8,7 +8,7 @@ use crate::api::dto::*;
|
||||
use crate::consts::*;
|
||||
use crate::engine::data_value::*;
|
||||
use crate::engine::filter_parser::*;
|
||||
use crate::engine::handle::*;
|
||||
use crate::engine::actor::primary::*;
|
||||
use crate::engine::index_key::*;
|
||||
use crate::engine::message::*;
|
||||
use crate::engine::metric_primitive::*;
|
||||
|
||||
@@ -0,0 +1,72 @@
|
||||
# 109 — Stop binding `ottyl mcp` to one session for its whole lifetime
|
||||
|
||||
## Summary
|
||||
|
||||
`ottyl mcp --session <name>` resolves the session name to a port
|
||||
once, at process startup, and then serves stdio JSON-RPC for the
|
||||
life of the process against that fixed URL. In practice this means
|
||||
every ottyl session an agent wants to talk to needs its own
|
||||
registered MCP server entry (e.g. `ottyl-hygienic-gyrfalcon`), and
|
||||
switching which running `ottyl` instance a client is looking at
|
||||
requires killing that entry and re-registering a new one — which,
|
||||
for MCP clients that only read server config at startup, means the
|
||||
whole host process needs restarting before the new tools are even
|
||||
callable. This got hit directly: an agent needed to switch from an
|
||||
old dead session (`enormous-warthog`) to a new one
|
||||
(`hygienic-gyrfalcon`) mid-conversation and had no way to do it
|
||||
without editing MCP config and waiting for a restart.
|
||||
|
||||
## Where
|
||||
|
||||
- `src/cli/mod.rs:43-49` — `Command::Mcp { session: Option<String>,
|
||||
port: Option<u16> }`, the CLI surface.
|
||||
- `src/cli/mod.rs:171-195` (`run_mcp`) — resolves `session` to a port
|
||||
via `sessions::resolve_session` exactly once, builds one
|
||||
`OttylMcp::new(base_url)`, then blocks forever on
|
||||
`service.waiting()`. There is no re-resolution after startup.
|
||||
- `src/api/mcp_server.rs:254-282` — `OttylMcp` stores `base_url:
|
||||
String` as a plain field set once in `new()`; every `#[tool]`
|
||||
method builds request URLs via `self.url(path)`
|
||||
(`mcp_server.rs:282-284`), so the fixed base URL is threaded
|
||||
through every single tool call.
|
||||
- `src/cli/sessions.rs` — `SessionInfo { name, port, pid }` and
|
||||
`resolve_session`/`list_sessions`, the registry this would need to
|
||||
re-query if the bound session can change after startup.
|
||||
|
||||
## Direction
|
||||
|
||||
Two ways to remove the "pick a session forever at startup" behavior;
|
||||
pick one during implementation rather than building both:
|
||||
|
||||
1. **Request/response CLI, no long-running process.** Add a CLI
|
||||
subcommand (e.g. `ottyl call <session> <tool> <json-args>`) that
|
||||
resolves the session, makes the one HTTP call, prints the result,
|
||||
and exits — no `rmcp`/stdio JSON-RPC server at all. Simplest fix
|
||||
for the "MCP config never changes, session does" mismatch, but
|
||||
loses whatever the MCP client gives you over a bare CLI call
|
||||
(tool discovery/schema, a persistent connection) — check whether
|
||||
that matters to how this is actually invoked before going this
|
||||
route.
|
||||
2. **Long-running MCP server, session switchable at runtime.** Keep
|
||||
`ottyl mcp` as a stdio server, but stop fixing `base_url` at
|
||||
construction: either (a) accept a `session` (or `port`) argument
|
||||
on every tool call, resolved against `sessions::resolve_session`
|
||||
per-call instead of once in `run_mcp`, with the constructor's
|
||||
`--session`/`--port` becoming just a default; or (b) add an
|
||||
explicit `switch_session` tool that mutates the stored target
|
||||
(`base_url` would need to move from a plain field to something
|
||||
mutable, e.g. `Mutex<String>` or resolved fresh from the registry
|
||||
on every call from a stored session name instead of a
|
||||
pre-resolved URL).
|
||||
|
||||
## Scope
|
||||
|
||||
- Land one of the two directions above for `ottyl mcp` /
|
||||
`src/api/mcp_server.rs`; out of scope to build both.
|
||||
- If going with direction 2, update `SKILL.md`'s "Connect an MCP
|
||||
client" section, since it currently documents the
|
||||
resolve-once-at-startup model.
|
||||
- Out of scope: changing anything about the underlying `/api/...`
|
||||
HTTP routes themselves, or the session registry format in
|
||||
`src/cli/sessions.rs` — this is purely about how the MCP-facing
|
||||
process picks and holds (or doesn't hold) its target session.
|
||||
@@ -0,0 +1,72 @@
|
||||
# 110 — Clicking a legend entry should focus that series, fading the rest
|
||||
|
||||
## Summary
|
||||
|
||||
`PanelChart`'s line and pie charts render a recharts `<Legend>`
|
||||
(`webapp/src/components/PanelChart.tsx:134`, `:190`) but it's purely
|
||||
decorative — clicking an entry does nothing. Bar charts have no
|
||||
`<Legend>` at all (`:151-168`), since each bar is its own labeled
|
||||
category on the x-axis rather than a separate series.
|
||||
|
||||
Add click-to-focus: clicking a legend entry marks that series
|
||||
"focused"; any focused series stay at full opacity/stroke while every
|
||||
other series fades. Clicking the same entry again (or clicking empty
|
||||
space in the legend, per whatever recharts exposes) clears focus and
|
||||
returns everyone to equal opacity — i.e. "no focused series" means
|
||||
"all at the same opacity", not "all faded".
|
||||
|
||||
## Where
|
||||
|
||||
- `webapp/src/components/PanelChart.tsx:116-148` — line chart. Series
|
||||
are recharts `<Line>` elements keyed by `keys` (from `toLineData`,
|
||||
`:60-78`), colored via `COLORS[i % COLORS.length]` (`:140`). Fading
|
||||
a line means varying `strokeOpacity` (or swapping `stroke` for a
|
||||
`color-mix`'d faded variant) per key based on focus state.
|
||||
- `webapp/src/components/PanelChart.tsx:170-193` — pie chart. Slices
|
||||
are `<Cell>` elements (`:179-181`) keyed by row index into `rows`
|
||||
(from `squashRows`, `:50-57`); fading a slice means varying its
|
||||
`fillOpacity`.
|
||||
- `webapp/src/components/PanelChart.tsx:134`, `:190` — the two
|
||||
`<Legend>` usages. recharts' `<Legend>` takes an `onClick` handler
|
||||
on its payload entries (`(entry) => void`, where `entry.dataKey` /
|
||||
`entry.value` identifies which series was clicked) — this is the
|
||||
hook point rather than building a custom legend from scratch.
|
||||
- Bar charts (`:151-168`) have no legend to click, since bars aren't
|
||||
named series here — out of scope for this ticket unless the
|
||||
direction chosen below turns out to need bar-chart parity (see
|
||||
Scope).
|
||||
|
||||
## Direction
|
||||
|
||||
- Add focus state as a `Set<string>` (or single nullable string, if
|
||||
only one series can be focused at a time — decide during
|
||||
implementation) of series keys, local to `PanelChart` via
|
||||
`useState`. Reset it when `series` changes identity to avoid stale
|
||||
focus surviving a query change onto a chart with different series.
|
||||
- Wire the `<Legend>`'s `onClick` to toggle membership in that set.
|
||||
- Compute per-series opacity from the set: full opacity if the set is
|
||||
empty or the series is a member, faded (e.g. `0.25`) otherwise.
|
||||
Thread that into `<Line stroke=...>` / `<Cell fill=...>` via
|
||||
`strokeOpacity`/`fillOpacity` rather than swapping the base color,
|
||||
so the existing `COLORS` palette and its `color-mix` theming stay
|
||||
untouched.
|
||||
- The legend entries themselves should visually reflect focus too
|
||||
(e.g. dim the faded entries' swatch/text) so it's clear what's
|
||||
focused without hunting across the plot — recharts' legend
|
||||
`formatter` prop can restyle each entry's label.
|
||||
|
||||
## Scope
|
||||
|
||||
- Line and pie charts only, matching where `<Legend>` already
|
||||
renders. Bar charts stay as-is — no legend, no per-bar fade — unless
|
||||
investigation shows recharts' bar `<Legend>` support is trivial to
|
||||
add alongside this; if so, note it as a follow-up rather than
|
||||
folding scope creep into this ticket.
|
||||
- Out of scope: persisting focus state across re-renders driven by
|
||||
polling (`Dashboards.tsx`'s `PanelPreview`, `:46-92`, re-fetches on
|
||||
every poll tick) — focus should survive a data refresh as long as
|
||||
the series identity (legend key) is unchanged, but there's no need
|
||||
to persist it to `localStorage` or across a page reload.
|
||||
- Out of scope: multi-select "compare exactly these two series" UX
|
||||
beyond simple toggle-membership — keep it to click-to-add/click-to-
|
||||
remove from the focused set.
|
||||
@@ -0,0 +1,51 @@
|
||||
# 111 — Legend must scroll instead of overflowing the panel
|
||||
|
||||
## Summary
|
||||
|
||||
`PanelChart`'s recharts `<Legend>` (`webapp/src/components/
|
||||
PanelChart.tsx:134`, `:190`) has no height constraint or scroll
|
||||
container. A panel with many series (e.g. a high-cardinality PromQL
|
||||
query, or the post-`squashRows` `MAX_KEPT_SERIES` = 7 kept rows plus
|
||||
"Other") wraps the legend across as many lines as it needs, growing
|
||||
the panel's rendered height and pushing surrounding dashboard layout
|
||||
around rather than staying within the chart's fixed footprint.
|
||||
|
||||
## Where
|
||||
|
||||
- `webapp/src/components/PanelChart.tsx:117`, `:154`, `:176` — each
|
||||
chart type wraps its `<ResponsiveContainer>` at a fixed `height={220}`,
|
||||
but the `<Legend>` inside it (`:134`, `:190`) isn't given its own
|
||||
bounded, scrollable region — recharts lays it out in the space left
|
||||
after the plot area, which can push the container's effective
|
||||
rendered height past 220 when the legend wraps to multiple rows.
|
||||
- `webapp/src/components/Dashboards.tsx:46-92` (`PanelPreview`) is the
|
||||
only caller of `PanelChart` today — worth checking that whatever
|
||||
fixed-height/scroll wrapper gets added here doesn't fight with
|
||||
`PanelPreview`'s own loading/error states (`:85-90`), which render
|
||||
outside `PanelChart` at a different fixed height (`h-48`).
|
||||
|
||||
## Direction
|
||||
|
||||
- recharts' `<Legend>` supports a `wrapperStyle` prop (already used
|
||||
for `fontSize`, `:134`/`:190`) — extending it with a fixed
|
||||
`maxHeight` and `overflowY: 'auto'` is the natural fix and avoids
|
||||
building a custom legend renderer just for this.
|
||||
- Verify this doesn't conflict with the click-to-focus work in
|
||||
[[110-legend-click-to-focus-series]] — that ticket also touches the
|
||||
`<Legend>` props (`onClick`, `formatter`) on these same two call
|
||||
sites, so land whichever lands second by rebasing onto the other's
|
||||
`<Legend>` props rather than both independently restructuring it.
|
||||
- Decide a concrete max height (e.g. two or three legend rows' worth)
|
||||
that keeps the panel's total footprint predictable in a dashboard
|
||||
grid; check `Dashboards.tsx`'s panel grid/card layout for whatever
|
||||
existing panel height convention should inform the number.
|
||||
|
||||
## Scope
|
||||
|
||||
- Line and pie charts only, matching where `<Legend>` already
|
||||
renders — same boundary as [[110-legend-click-to-focus-series]].
|
||||
- Out of scope: virtualizing the legend list for very large series
|
||||
counts — `squashRows` (`PanelChart.tsx:50-57`) already caps bar/pie
|
||||
series at `MAX_KEPT_SERIES` (7) + "Other", and line charts have no
|
||||
equivalent cap today, but adding one is a separate concern from
|
||||
making the legend scrollable.
|
||||
@@ -0,0 +1,80 @@
|
||||
# 112 — Let a single dashboard panel pop out into its own window
|
||||
|
||||
## Summary
|
||||
|
||||
There's already a pop-out mechanism for whole primary sections (log,
|
||||
params, metrics, dashboards) — `PopOutButton` opens a new tab at
|
||||
`?section=<id>` and `App.tsx` reads that query param at startup to
|
||||
render a standalone, full-page version of just that section (see
|
||||
`webapp/src/components/CollapsibleSection.tsx:193-232` and
|
||||
`webapp/src/App.tsx:129-140`, `:372-376`). That grain is a whole
|
||||
section, not a single panel — the Dashboards section can only be
|
||||
popped out in its entirety (all panels of the active dashboard, or
|
||||
all dashboards stacked via `AllDashboardsView`,
|
||||
`Dashboards.tsx:107-135`). This ticket adds the finer-grained version:
|
||||
popping out one panel from one dashboard into its own window,
|
||||
independent of the rest of that dashboard.
|
||||
|
||||
## Where
|
||||
|
||||
- `webapp/src/components/Dashboards.tsx:46-92` (`PanelPreview`) is
|
||||
the component that would need to render standalone, full-page, the
|
||||
same way `StandaloneLog`/`StandaloneParams`/`StandaloneRanged` do
|
||||
today for whole sections (referenced in `App.tsx:373-376`, exact
|
||||
component locations elsewhere in `Dashboards.tsx`/`Metrics.tsx` per
|
||||
the skeleton read during this ticket's writeup — re-read
|
||||
`Dashboards.tsx` in full before implementing, since it was read via
|
||||
repowise's skeleton view here and several ranges (`:1-45`, `:136-
|
||||
434`) were elided).
|
||||
- `webapp/src/components/CollapsibleSection.tsx:193-206`
|
||||
(`PopOutSectionId`, `popOutUrl`) — the existing section-grain
|
||||
pop-out routing. A panel-grain pop-out needs to identify a specific
|
||||
`(dashboardId, panelId)` pair in the URL, not just a section name —
|
||||
either extend this query-param scheme with `dashboardId`/`panelId`
|
||||
params alongside `section=dashboards`, or give panels their own
|
||||
distinct query shape if reusing `PopOutSectionId` doesn't fit
|
||||
cleanly.
|
||||
- `webapp/src/App.tsx:129-140` (`parsePopOutQuery`), `:372-376` (the
|
||||
dispatch on parsed `section`) — where the new panel-grain case needs
|
||||
a branch, reading whatever new params are added above.
|
||||
- Each `Panel` (`webapp/src/lib/api.ts`'s `Panel` type, imported at
|
||||
`Dashboards.tsx:19`) needs a stable identifier already, or one to
|
||||
add, so a popped-out URL can point back at "this exact panel of
|
||||
this exact dashboard" — check whether `Panel` already carries an
|
||||
`id` field before inventing a new addressing scheme.
|
||||
|
||||
## Direction
|
||||
|
||||
- Mirror the existing section pop-out exactly: a small pop-out button
|
||||
per panel (using or extending `PopOutButton`,
|
||||
`CollapsibleSection.tsx:209-232`) that opens `?section=dashboards&
|
||||
dashboardId=<id>&panelId=<id>`, plus range params the same way
|
||||
`popOutUrl` already carries over a rolling preset or fixed range
|
||||
(`CollapsibleSection.tsx:196-206`).
|
||||
- The standalone page for this case renders just that one panel's
|
||||
`PanelPreview` (or a thin wrapper around it) full-page — no
|
||||
dashboard chrome, no other panels — analogous to how
|
||||
`StandaloneRanged` (`App.tsx:179-...`) drives `LogTimeline`'s
|
||||
picker UI on its own poll cadence for the metrics/dashboards
|
||||
standalone views.
|
||||
- The popped-out panel window should keep polling independently
|
||||
(same `POLL_MS` pattern `PanelPreview` already uses,
|
||||
`Dashboards.tsx:52-55`) rather than freezing at open time.
|
||||
|
||||
## Scope
|
||||
|
||||
- One panel popping into a new browser tab/window, matching the
|
||||
existing section pop-out's mechanism (new tab via `window.open`,
|
||||
not an in-app floating/detached window widget) — stay consistent
|
||||
with the pattern already established for log/params/metrics/
|
||||
dashboards rather than introducing a second, different pop-out
|
||||
mechanism.
|
||||
- Out of scope: editing a panel's query/chart type from the popped-out
|
||||
view — read-only preview only, matching what pop-out gives you for
|
||||
the other sections today.
|
||||
- Out of scope: [[110-legend-click-to-focus-series]] and
|
||||
[[111-scrollable-panel-legend]] — this ticket is purely about the
|
||||
window/routing mechanism; the popped-out panel should get those
|
||||
legend behaviors for free once they land in `PanelChart`, since
|
||||
it's the same component, but implementing them isn't this ticket's
|
||||
job.
|
||||
Reference in New Issue
Block a user