Add Bonfire: multi-campfire river view, front page, and ticker

[jeremy]

Summary

Adds the Bonfire feature: a multi-campfire experience with three UI components — River (unified conversation stream), Front Page (activity overview), and Ticker (ambient status bar).

What's included

• Data foundation: RoomID, segmenter with 6-signal continuity scoring, room/read/mixer persistence stores, text utilities
• LLM summarizer: Provider interface with Anthropic, OpenAI, Ollama, and Apple Foundation Models (macOS 26+) backends; extractive fallback when no LLM configured; disk + memory cache
• Hub pools: BonfireRooms (bookmarked-project filtering with override contract), BonfireLines (per-room adaptive polling), BonfireDigest (self-bootstrapping for fresh sessions)
• Multiplexer support: tmux/zellij detection, bonfire split and bonfire layout CLI commands
• Navigation: ctrl+g → Front Page, chat URL routing, :bonfire/:front-page palette actions
• River view: segmented multi-room display with colored gutters, inline composer with Tab room cycling, briefing mode (idle detection + gap summaries), mixer overlay (per-room volume/polling control)
• Front Page: burst-tiered room list (Hot/Warm/Quiet), navigate to campfire on Enter
• Ticker chrome: ambient last-message display across all rooms, flash on new messages
• Config: llm_provider, llm_endpoint, llm_api_key, llm_model, llm_max_concurrent settings

Commits

1. Data foundation (types, segmenter, stores)
2. LLM summarizer with multi-provider support
3. Hub pools, session wiring, LLM config, room colors
4. Multiplexer detection and bonfire CLI commands
5. Navigation plumbing (URL routing, keymap, view targets)
6. Views and chrome (River, Front Page, Ticker)

Test plan

• make check passes (fmt, vet, lint, unit tests, e2e tests)
• basecamp tui → ctrl+g → Front Page shows burst-tiered rooms → Enter opens campfire
• River view renders segmented multi-room lines with colored gutters
• Composer sends messages to active room (Tab cycles)
• B toggles briefing mode with gap summaries
• M toggles mixer overlay; volume changes affect poll rates
• Ticker appears in status area when rooms are configured
• bonfire split <url> opens pane in tmux/zellij
• config set llm_provider ollama enables LLM summaries

---

Summary by cubic

Introduces Bonfire: a multi-campfire experience with River, Front Page, and an interactive Bonfire Sidebar to follow several chats at once. Bonfire is opt-in and gated behind the experimental.bonfire flag.

• New Features

  • River view with room-colored gutters, inline composer (Tab cycles), per-room adaptive polling, and persisted room/mixer/read state.
  • Briefing mode for gap summaries; mixer overlay to set room volume (5s/15s/30s/60s/off).
  • Front Page overview with inline activity dots and relative times; Enter opens a room.
  • Bonfire Sidebar (replaces the ticker): compact, interactive chat/ping panel (ctrl+b to toggle). Groups CAMPFIRES and PINGS, shows latest messages, scrolls, and opens rooms on Enter.
  • LLM summarizer with Anthropic/OpenAI/Ollama/Apple providers; extractive fallback and disk+memory cache.
  • tmux/zellij support with bonfire split and bonfire layout for pane orchestration.
  • Navigation: ctrl+g, :bonfire, :front-page, and chat URL routing (/chats and /chats/.../lines).
  • Config: 'llm' is shorthand for 'llm_provider' in config set/unset.
  • Feature flag: enable Bonfire via basecamp config set experimental.bonfire true --global; gates ctrl+g, sidebar, digest polling, palette actions, BonfireRooms invalidation, bonfire CLI, and hides the ctrl+g keybinding from help/status when off.

• Bug Fixes

  • River lifecycle: tear down stale rooms on updates, preserve focused room across rebuilds, clamp active room/mixer cursor when a room disappears.
  • Polling and focus: muted rooms no longer re-schedule, timers re-arm after Tab cycling, update poll focus on initial load.
  • Cross-account: scope recent projects per account, de-duplicate recents by (ID, AccountID), and distribute the room cap round-robin across accounts.
  • LLM/config: validate llm_provider; handle 'auto' explicitly; unknown providers fail closed; 30s timeouts; cap error reads; use session-scoped context so summaries cancel on account switch; context-aware semaphore prevents blocking after cancel.
  • Summaries: briefing toggle kicks off LLM gap summaries for existing segments; atomic writes in summary cache.
  • Text: strip leading '@' in mentions; normalize non-breaking spaces in River text.
  • Multiplexer/layout: add cache_dir guard for layout list/save/load; guard empty tmux output.
  • Stability/UI: deterministic watermark rendering; account for mixer height; fix RoomID color hashing on 32-bit; clamp sidebar scroll offset; clear stale pointers in segmenter; River init uses FetchIfStale; Hub digest populates RoomName with ProjectName; Front Page shows ◐ for warm tier.
  • Config/flags: load experimental flags from disk across restarts; track per-feature provenance in config show; redact llm_api_key in output; regression test for experimental flag provenance.
  • Apple provider: implement Complete in non-darwin stub to restore Linux/CI builds.
  • Lint/cleanup: gofmt alignment, remove unused params, add guarded int→uint32 conversion nolint, consistent receiver names in Config.

^{Written for commit 2b6df85. Summary will update on new commits.}

[cubic-dev-ai]

24 issues found across 43 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="internal/tui/workspace/views/river.go">

<violation number="1" location="internal/tui/workspace/views/river.go:603">
P1: Unconditional `break` outside the `if` block causes nondeterministic watermark rendering. Only one arbitrary watermark is checked per segment due to Go's random map iteration order. If that watermark doesn't match, no others are examined. Move the `break` inside the `if` (or refactor to select a single representative watermark before the segment loop).</violation>
</file>

<file name="internal/tui/workspace/summarize/summarize_test.go">

<violation number="1" location="internal/tui/workspace/summarize/summarize_test.go:236">
P2: Bug: `filepath.Glob` only returns an error for malformed patterns, not for missing files. This test will always pass even if the file doesn't exist. Use `os.Stat` instead to verify the file was written.</violation>
</file>

<file name="internal/tui/workspace/summarize/prompt.go">

<violation number="1" location="internal/tui/workspace/summarize/prompt.go:28">
P2: String concatenation with `+=` inside a loop is O(n²) in Go. Use `strings.Builder` for efficient prompt assembly, especially since segment lists can be large.</violation>
</file>

<file name="internal/hostutil/multiplexer.go">

<violation number="1" location="internal/hostutil/multiplexer.go:74">
P2: `strings.Split("", "\n")` returns `[""]`, not `[]string{}`. When tmux output is empty, this returns a slice with one empty string instead of an empty slice, which will mislead callers checking the length.</violation>
</file>

<file name="internal/tui/workspace/summarize/provider_apple.go">

<violation number="1" location="internal/tui/workspace/summarize/provider_apple.go:21">
P2: `maxTokens` is accepted but silently ignored, unlike every other `Provider` implementation. The FoundationModels `LanguageModelSession` supports generation configuration — pass the token limit through to the Swift script (e.g. via a CLI argument or an additional stdin field) so the Apple backend respects the same contract as the Anthropic, OpenAI, and Ollama providers.</violation>
</file>

<file name="internal/tui/workspace/data/textutil.go">

<violation number="1" location="internal/tui/workspace/data/textutil.go:11">
P2: Duplicated `reMention` regex pattern — the identical regex already exists in `internal/richtext/richtext.go`. Consider exporting the regex (or an `ExtractMentions` helper) from the `richtext` package to avoid maintaining the same pattern in two places.</violation>
</file>

<file name="internal/config/config.go">

<violation number="1" location="internal/config/config.go:234">
P2: Silently dropping `llm_api_key` from local/repo config with no warning. All other restricted keys (`base_url`, `llm_provider`, `llm_endpoint`, `default_profile`, `profiles`) emit a stderr warning when ignored, but `llm_api_key` is silently discarded. A user who puts their key in a repo config will have no idea why it's not working. Add a warning guiding them to use global config or the `BASECAMP_LLM_API_KEY` env var.</violation>
</file>

<file name="internal/tui/workspace/summarize/provider_openai.go">

<violation number="1" location="internal/tui/workspace/summarize/provider_openai.go:70">
P2: Unbounded `io.ReadAll` on error responses could lead to excessive memory allocation if the server returns a very large body. Use `io.LimitReader` to cap the read size (e.g., 1 MB).</violation>
</file>

<file name="internal/tui/workspace/summarize/summarize.go">

<violation number="1" location="internal/tui/workspace/summarize/summarize.go:121">
P2: External LLM API call uses `context.Background()` with no timeout. If the LLM endpoint is slow or unresponsive, this goroutine hangs indefinitely, permanently consuming a semaphore slot. With `maxConcurrent` defaulting to 3, a few stalled requests will block all future LLM summaries. Use `context.WithTimeout` to bound the wait.</violation>
</file>

<file name="internal/commands/bonfire.go">

<violation number="1" location="internal/commands/bonfire.go:115">
P1: User-created layouts are stored in the cache directory, which is documented as "Ephemeral tool data" and can be cleared at any time. Named layouts saved by the user via `bonfire layout save` are persistent configuration data and should be stored in the config directory (e.g., `app.Config.ConfigDir`) to avoid silent data loss when the cache is purged.</violation>
</file>

<file name="internal/tui/workspace/chrome/ticker.go">

<violation number="1" location="internal/tui/workspace/chrome/ticker.go:108">
P1: ANSI-unsafe string truncation: `utf8.RuneCountInString` and rune slicing don't account for ANSI escape codes injected by `lipgloss.Render()`. This will miscalculate visible width and can slice through escape sequences, producing garbled output. Use `ansi.StringWidth()` and `ansi.Truncate()` from the already-imported `charmbracelet/x/ansi` package.</violation>
</file>

<file name="internal/tui/workspace/summarize/provider.go">

<violation number="1" location="internal/tui/workspace/summarize/provider.go:30">
P2: Unrecognized `providerName` values silently fall back to auto-detection, masking configuration errors. A typo like `"anthrpoic"` would skip the intended provider and silently use whatever `autoDetectProvider()` finds. Return `nil` for unknown names so the caller can surface the misconfiguration.</violation>
</file>

<file name="internal/tui/workspace/data/mixerstore.go">

<violation number="1" location="internal/tui/workspace/data/mixerstore.go:59">
P2: `Save` overwrites disk state without read-merge-write, unlike sibling stores (`RoomStore`, `ReadTracker`). If multiple TUI instances modify volumes for different rooms, the last writer silently drops the other instance's settings. Consider reading existing disk data under the lock, merging per-room volumes (e.g., latest-write-wins), and then writing — consistent with the pattern in `roomstore.go` and `readtracker.go`.</violation>
</file>

<file name="internal/tui/workspace/data/roomstore.go">

<violation number="1" location="internal/tui/workspace/data/roomstore.go:24">
P2: `UpdatedAt` is a `string` compared with `>=` for last-writer-wins semantics. This only works for lexicographically sortable formats (RFC 3339). Use `time.Time` instead so comparison is always correct regardless of format, and JSON serialization defaults to RFC 3339.</violation>
</file>

<file name="internal/tui/workspace/summarize/cache.go">

<violation number="1" location="internal/tui/workspace/summarize/cache.go:20">
P2: Comment claims "in-memory LRU" but the eviction policy is oldest-by-creation, not least-recently-used. `evictIfNeeded` selects the entry with the earliest `CreatedAt`, and `Get` never updates this timestamp on access. A true LRU would track last-access time and evict the entry accessed longest ago. Either rename the comment to reflect the actual policy (e.g., "oldest-first eviction") or add a `LastAccess` field updated on every `Get` hit.</violation>
</file>

<file name="internal/tui/workspace/data/readtracker.go">

<violation number="1" location="internal/tui/workspace/data/readtracker.go:162">
P1: **`acquireLock` silently proceeds without the file lock on timeout/failure, risking data loss.** When the lock cannot be acquired (timeout or contention), `(nil, nil)` is returned and `Flush()` performs the entire read-merge-write cycle unprotected. With multiple bonfire panes in tmux/zellij, this creates a TOCTOU race that can silently lose read-position updates.

Return an error instead of `(nil, nil)` so `Flush()` retries or fails visibly rather than proceeding without mutual exclusion.</violation>
</file>

<file name="internal/tui/workspace/summarize/fallback.go">

<violation number="1" location="internal/tui/workspace/summarize/fallback.go:129">
P2: `truncateRunes` returns "…" (1 rune) when `maxRunes <= 0`, violating the caller's character budget. This is reachable from `extractZoom200` when `available` is 1 (`half = 1/2 = 0`). Guard `maxRunes <= 0` separately to return `""`.</violation>
</file>

<file name="internal/tui/workspace/data/pool.go">

<violation number="1" location="internal/tui/workspace/data/pool.go:252">
P2: `SetPollConfig` should reset `missCount` to zero. Without this, accumulated backoff from the previous config carries over, causing the new `PollBase` to be exponentially inflated. For example, when a user adjusts volume in the mixer to poll faster, the stale `missCount` can immediately push the interval to `PollMax`, making the config change appear to have no effect.</violation>
</file>

<file name="internal/tui/workspace/data/hub_global.go">

<violation number="1" location="internal/tui/workspace/data/hub_global.go:457">
P3: `digestResult.idx` is set but never read — entries are appended in arrival order and then sorted by `LastAtTS`. Remove the `idx` field if positional ordering isn't needed, or use it for positional insertion if it is.</violation>

<violation number="2" location="internal/tui/workspace/data/hub_global.go:487">
P2: Variable `r` (the goroutine's `BonfireRoomConfig` parameter) is shadowed by `r := []rune(content)` in the if-init. Rename either the rune slice or the closure parameter to avoid confusion and `go vet -shadow` warnings.</violation>
</file>

<file name="internal/tui/workspace/summarize/provider_ollama.go">

<violation number="1" location="internal/tui/workspace/summarize/provider_ollama.go:21">
P1: Missing default model: when `model` is empty (e.g. via `autoDetectProvider()` which passes `""`), the request body sends `"model": ""` to Ollama, which will reject it. The OpenAI sibling provider defaults to `"gpt-4o-mini"` in the same scenario. Add a fallback here (e.g. `"llama3"`) or return an error early.</violation>
</file>

<file name="internal/commands/config.go">

<violation number="1" location="internal/commands/config.go:287">
P1: Secret leak: `llm_api_key` value is echoed back unredacted in `config set` output (both JSON `value` field and summary line), while `config show` properly redacts it via `redactSecret`. Add a dedicated switch case to redact the output value.</violation>
</file>

<file name="internal/tui/workspace/workspace.go">

<violation number="1" location="internal/tui/workspace/workspace.go:429">
P1: Missing `relayout()` when ticker transitions between active/inactive. When the first digest data arrives, `ticker.Active()` changes from false to true, which changes `chromeHeight()` from 3 to 4 and should reduce `viewHeight()` by 1. Without a relayout, all views keep their stale dimensions and the total rendered height exceeds the terminal, causing visual overflow. This matches the existing pattern for `showMetrics` toggle which calls `w.relayout()` after changing state.</violation>
</file>

<file name="internal/tui/workspace/data/types.go">

<violation number="1" location="internal/tui/workspace/data/types.go:273">
P2: On 32-bit architectures, `int(h.Sum32())` can overflow to a negative value, making the modulo result negative. This would panic when used as a slice index. Perform the modulo before the `int` conversion to keep the arithmetic unsigned.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b6887af727

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

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".

[Copilot]

Pull request overview

Adds the “Bonfire” multi-campfire experience to the TUI, including River + Front Page views, an ambient ticker, supporting data pools/stores, and optional LLM-backed summarization.

Changes:

• Introduces Bonfire UI (River + Front Page) and a global ticker with ambient digest polling.
• Adds Bonfire data foundation: RoomID, segmenter + text utilities, persistence stores (read state, mixer volumes, room overrides), and Hub pools for rooms/lines/digest.
• Adds summarization subsystem with multi-provider support (Anthropic/OpenAI/Ollama/Apple) plus extractive fallback and disk+memory caching; wires config/env support and URL routing.

Reviewed changes

Copilot reviewed 43 out of 43 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
internal/tui/workspace/workspace.go	Adds ticker chrome, dynamic chrome height, and ambient Bonfire digest polling + keybinding routing.
internal/tui/workspace/views/river.go	New River view: multi-room segmented stream, composer, briefing mode gap summaries, mixer overlay + adaptive polling.
internal/tui/workspace/views/frontpage.go	New Front Page view: burst-tiered room overview with navigation into campfire.
internal/tui/workspace/summarize/zoom.go	Defines zoom buckets and bucketing helper.
internal/tui/workspace/summarize/summarize.go	Summarizer core: cache lookup + extractive fallback + async provider completion.
internal/tui/workspace/summarize/provider.go	Provider interface and provider auto-detection logic.
internal/tui/workspace/summarize/provider_anthropic.go	Anthropic Messages API backend.
internal/tui/workspace/summarize/provider_openai.go	OpenAI-compatible chat completions backend.
internal/tui/workspace/summarize/provider_ollama.go	Ollama local backend.
internal/tui/workspace/summarize/provider_apple.go	macOS Foundation Models backend via swift.
internal/tui/workspace/summarize/provider_apple_stub.go	Non-darwin stub for Apple provider.
internal/tui/workspace/summarize/prompt.go	Prompt construction with hint-specific instructions.
internal/tui/workspace/summarize/fallback.go	Extractive summarization fallback implementation.
internal/tui/workspace/summarize/cache.go	Disk-backed cache with in-memory eviction.
internal/tui/workspace/summarize/summarize_test.go	Unit tests for zoom bucketing, fallback, cache keying, cache persistence/TTL, etc.
internal/tui/workspace/session.go	Wires hub room store + summarizer initialization from config.
internal/tui/workspace/msg.go	Adds ViewBonfire/ViewFrontPage targets and exposes Bonfire-related data aliases.
internal/tui/workspace/keymap.go	Adds global ctrl+g Bonfire binding and action mapping.
internal/tui/workspace/data/types.go	Adds RoomID + Bonfire room/digest types and deterministic color indexing.
internal/tui/workspace/data/textutil.go	Adds mention extraction, tokenization, similarity, question detection, and tag stripping helpers.
internal/tui/workspace/data/segment.go	Adds segmenter implementation with 6-signal continuity scoring and stale sealing.
internal/tui/workspace/data/segment_test.go	Adds tests for segmenter heuristics and textutil helpers.
internal/tui/workspace/data/roomstore.go	Adds disk-persisted include/exclude overrides with locking + atomic writes.
internal/tui/workspace/data/readtracker.go	Adds disk-persisted per-room last-read tracking with locking + atomic writes.
internal/tui/workspace/data/mixerstore.go	Adds disk-persisted per-room mixer volume settings with locking + atomic writes.
internal/tui/workspace/data/pool.go	Adds SetPollConfig to support view-driven polling config changes.
internal/tui/workspace/data/hub_global.go	Adds BonfireRooms/BonfireLines/BonfireDigest global pools.
internal/tui/workspace/data/hub.go	Adds hub RoomStore plumbing + shares campfire line mapping helper.
internal/tui/workspace/chrome/ticker.go	New ticker chrome component with flash-on-new-message behavior.
internal/tui/workspace/actions.go	Adds palette actions :bonfire and :front-page.
internal/tui/theme.go	Adds theme support for RoomColors with configurable overrides.
internal/tui/styles.go	Extends Theme with RoomColors defaults.
internal/hostutil/multiplexer.go	Adds tmux/zellij detection and split/layout helpers.
internal/hostutil/multiplexer_test.go	Adds basic multiplexer tests.
internal/config/config.go	Adds LLM config fields, file/env loading rules, and defaults.
internal/commands/tui_url.go	Adds chat URL routing to ViewCampfire and parent chat extraction for nested line URLs.
internal/commands/tui_url_test.go	Updates/extends URL parsing tests for chat routing (including localhost dev URL case).
internal/commands/tui.go	Extends view factory for Bonfire views.
internal/commands/bonfire.go	Adds bonfire command group (split/layout save/load/list).
internal/commands/commands.go	Registers bonfire in the command catalog.
internal/commands/commands_test.go	Updates catalog test to include NewBonfireCmd().
internal/cli/root.go	Adds Bonfire command to root CLI.
internal/commands/config.go	Exposes LLM settings in config show/set, adds redaction helper, and authority key handling.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[cubic-dev-ai]

1 issue found across 4 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="internal/tui/workspace/data/textutil.go">

<violation number="1" location="internal/tui/workspace/data/textutil.go:109">
P2: Use `html.UnescapeString()` from the standard library instead of manually decoding a hardcoded subset of HTML entities. The current approach only handles 6 entities and will leave others (e.g., `&hellip;`, `&mdash;`, `&rsquo;`, `&#8212;`) as raw text in the river view.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

[Copilot]

Pull request overview

Copilot reviewed 43 out of 43 changed files in this pull request and generated 8 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[cubic-dev-ai]

1 issue found across 3 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="internal/tui/workspace/data/hub_global.go">

<violation number="1" location="internal/tui/workspace/data/hub_global.go:329">
P1: Fresh accounts with no bookmarks AND no recent project visits will see an empty Bonfire view. The old code treated all rooms as bookmarked in this case; the new code silently yields zero rooms when `recentFn` is nil or returns no matching IDs. Consider adding a final fallback (e.g., treat all rooms as bookmarked when both bookmarks and recents are empty).</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

[Copilot]

Pull request overview

Copilot reviewed 43 out of 43 changed files in this pull request and generated 4 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[cubic-dev-ai]

1 issue found across 7 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="internal/tui/workspace/views/bonfire_sidebar.go">

<violation number="1" location="internal/tui/workspace/views/bonfire_sidebar.go:172">
P1: Pressing `G` when there are no items sets `cursor` to `-1` (since `total - 1 = -1`). A subsequent `Enter` will pass `-1` to `itemAt()`, where `-1 < len(b.digests)` is `true` for an empty slice, causing an index-out-of-range panic.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

[Copilot]

Pull request overview

Copilot reviewed 43 out of 43 changed files in this pull request and generated 8 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 44 out of 44 changed files in this pull request and generated 6 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[cubic-dev-ai]

3 issues found across 22 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="internal/tui/workspace/views/river.go">

<violation number="1" location="internal/tui/workspace/views/river.go:946">
P2: Viewport height is not recalculated when the mixer is toggled. `SetSize` now makes the viewport height depend on `r.mixerActive`, but the mixer toggle (`M` key) doesn't update the viewport dimensions. This causes the view to overflow by one row when the mixer is activated, or waste a row when deactivated after a resize.</violation>
</file>

<file name="internal/tui/workspace/data/textutil.go">

<violation number="1" location="internal/tui/workspace/data/textutil.go:110">
P2: `html.UnescapeString` decodes `&nbsp;` to U+00A0 (non-breaking space), not a regular space. Go's `\s` regex doesn't match U+00A0, so the whitespace-collapsing step below will skip these characters, causing rendering issues in the river view. Add a replacement after unescaping to normalize non-breaking spaces.</violation>
</file>

<file name="internal/commands/bonfire.go">

<violation number="1" location="internal/commands/bonfire.go:115">
P2: The `CacheDir == ""` guard was added to `layout save` and `layout load` but not to `layout list`, which also calls `loadBonfireLayouts(app.Config.CacheDir)`. For consistency and to avoid reading from the current working directory when CacheDir is unconfigured, the same guard should be added to `newBonfireLayoutListCmd`.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

[Copilot]

Pull request overview

Copilot reviewed 47 out of 47 changed files in this pull request and generated 3 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[cubic-dev-ai]

3 issues found across 15 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="internal/tui/workspace/summarize/provider.go">

<violation number="1" location="internal/tui/workspace/summarize/provider.go:35">
P1: Unknown provider names should fall back to `autoDetectProvider()` rather than returning `nil`. Returning `nil` here means a simple typo in the provider name (e.g. `"antrhopic"`) silently disables all LLM summarization instead of gracefully discovering a locally available provider.

(Based on your team's feedback about falling back to autoDetectProvider for unknown provider names.) [FEEDBACK_USED]</violation>
</file>

<file name="internal/tui/workspace/data/segment.go">

<violation number="1" location="internal/tui/workspace/data/segment.go:285">
P2: Memory leak: in-place filtering of a pointer slice without nil-ing trailing elements. After filtering, the backing array still holds `*Segment` pointers past the new length, preventing GC of pruned segments and their `Lines` data. Nil out the stale slots before reassigning.</violation>
</file>

<file name="internal/commands/config.go">

<violation number="1" location="internal/commands/config.go:281">
P2: Inconsistent indentation in the `case "llm_provider"` block: the case body should be indented one additional tab level (4 tabs) to match all other cases in this switch, and the subsequent `case "llm_max_concurrent"` label lost a tab level. This would fail `gofmt`.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

[cubic-dev-ai]

1 issue found across 6 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="internal/commands/config.go">

<violation number="1" location="internal/commands/config.go:327">
P1: Experimental feature flags are written to the config file but never loaded back. `loadFromFile()` in `internal/config/config.go` manually extracts each known key from the JSON map but has no block for the `"experimental"` key. As a result, `cfg.Experimental` is always nil after loading, `IsExperimental()` always returns false, and `config show` never displays experimental flags — the entire feature is non-functional.

Add an `experimental` loading block in `loadFromFile()` (in `internal/config/config.go`), similar to how other map fields like `profiles` are handled.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

[Copilot]

Pull request overview

Copilot reviewed 48 out of 48 changed files in this pull request and generated 3 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[cubic-dev-ai]

1 issue found across 5 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="internal/config/config.go">

<violation number="1" location="internal/config/config.go:278">
P2: Missing `cfg.Sources["experimental"]` assignment. Every other config key in this function records its source for provenance tracking, but `experimental` does not. This makes it impossible to trace where an experimental flag came from when debugging config resolution.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

[Copilot]

Pull request overview

Copilot reviewed 48 out of 48 changed files in this pull request and generated 4 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[cubic-dev-ai]

1 issue found across 7 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="internal/tui/workspace/views/river.go">

<violation number="1" location="internal/tui/workspace/views/river.go:408">
P2: This uses the global (app-lifetime) context, but `r.session.Context()` should be used instead so that in-flight LLM summary requests are cancelled on account switch — matching the behavior of every other context usage in this file and the stated intent in the PR description. The global context only cancels on full shutdown, so summaries started under one account will continue running (and hold semaphore slots) after switching accounts.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

[Copilot]

Pull request overview

Copilot reviewed 49 out of 49 changed files in this pull request and generated 2 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[pomarie]

Sorry Jeremy, cubic is really making you go through the rounds here :D