Skip to content

chore(backlog): complete inbox performance chores#94

Merged
bnema merged 8 commits into
mainfrom
chore/backlog-inbox-chores
Jun 22, 2026
Merged

chore(backlog): complete inbox performance chores#94
bnema merged 8 commits into
mainfrom
chore/backlog-inbox-chores

Conversation

@bnema

@bnema bnema commented Jun 22, 2026

Copy link
Copy Markdown
Owner

Summary

  • Complete the bundled chore backlog intake items in one branch.
  • Add repo agent instructions, atomic hook config writes, metadata watcher config refresh, and metadata reconcile lookup deduping.
  • Improve persisted-state migration/load reuse, git divergence/status reuse, and heat capture gating behavior.

Verification

  • go test ./...
  • make ci
  • Wrap-up review: no CRITICAL/HIGH/MEDIUM blockers
  • CodeRabbit findings addressed through the final trivial test-coverage fix

Summary by CodeRabbit

Release Notes

  • New Features

    • Added runtime configuration reload support for dynamic updates without restart.
  • Performance Improvements

    • Batch session path lookups reduce repeated queries.
    • Cached Git discovery/watch-target computations to minimize redundant operations.
  • Bug Fixes

    • Improved upstream divergence handling, including more reliable behavior when upstream refs are stale/missing.
    • Heat capture now correctly skips or persists based on the active configuration.
  • Refactor / Maintenance

    • Implemented atomic hook config writes to reduce risk of partial updates.

@coderabbitai

coderabbitai Bot commented Jun 22, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 284f4ae8-2de0-47b3-b549-91f64f859e8c

📥 Commits

Reviewing files that changed from the base of the PR and between 5fbc5b9 and 6ad5c5b.

📒 Files selected for processing (1)
  • internal/app/runtime_migrate_test.go

📝 Walkthrough

Walkthrough

The PR refactors git divergence targeting into a comparisonDivergence helper across both git adapters, switches all hook-install write paths to an atomic temp-file rename utility, centralises persisted-state construction/decoding into a new core/persisted package, adds a SessionHeatCaptureRequired predicate to skip unnecessary heat capture, and enhances MetadataService with batched session-path lookup, per-key memoisation of Git results, and runtime config reload. A new AGENTS.md file documents repository architecture and development guidance.

Changes

Git Divergence Refactoring

Layer / File(s) Summary
gitcli comparisonDivergence and --branch removal
internal/adapters/gitcli/git.go, internal/adapters/gitcli/git_test.go
Git.Status and Git.Divergence are rewritten using a new gitDivergenceResult struct and comparisonDivergence function that encodes which target was used and whether upstream was checked. git status --porcelain=v1 --branch drops the --branch flag. Tests assert exact rev-list target slices and absence of --branch.
gitgo comparisonDivergence and stale-upstream fallback
internal/adapters/gitgo/git.go, internal/adapters/gitgo/git_test.go
Git.status is rewritten with goDivergenceResult, comparisonDivergence, and divergenceResultForTarget. UpstreamConfigured is derived from checkedUpstream/targetIsUpstream flags or a fallback upstreamDivergence call. A new integration test simulates a deleted remote-tracking ref and asserts default-comparison fallback.

Atomic Hook File Writes

Layer / File(s) Summary
writeHookFileAtomic implementation
internal/app/hooks_atomic.go
New file adds writeHookFileAtomic (permission selection, os.CreateTemp in target dir, write/chmod/fsync/close, deferred temp removal on error, os.Rename) and syncDirBestEffort.
Hook write call sites and failure/permission tests
internal/app/hooks.go, internal/app/hooks_json.go, internal/app/hooks_special.go, internal/app/hooks_test.go
Every os.WriteFile call in hook install/uninstall paths is replaced with writeHookFileAtomic. Two new tests verify that a failed os.CreateTemp does not clobber existing config content and that existing file permissions (0600) are preserved after a successful install.

Persisted State Helpers, Store Migration, and Runtime Migration

Layer / File(s) Summary
core/persisted lifecycle helpers
internal/core/persisted/state.go
New EmptyState(), DecodeState([]byte), and InitializeState(*ports.PersistedState) functions centralise map/slice nil-to-empty initialisation for ports.PersistedState.
Store.Load delegates to persisted helpers
internal/adapters/storefs/store.go, internal/adapters/storefs/store_test.go
Store.Load replaces inline json.Unmarshal + initializeMaps with persisted.EmptyState and persisted.DecodeState. The local initializeMaps function is removed. Tests call persisted.InitializeState on expected values before comparison.
ensureRuntimeStateMigratedAndLoad and tri-state helpers
internal/app/runtime_migrate.go, internal/app/runtime_migrate_test.go
ensureRuntimeStateMigrated becomes a thin wrapper over a new internal function returning (state, loaded, err). Migration logic adopts a reusablePersistedState tri-state struct and helper functions. Four new tests cover current-state return, candidate migration, default initialised-state return, and malformed-state handling.
loadSidebarState uses ensureRuntimeStateMigratedAndLoad
internal/app/session_order.go
loadSidebarState calls ensureRuntimeStateMigratedAndLoad and returns the migrated state directly when loaded is true, eliminating the unconditional follow-up store.Load.

SessionHeatCaptureRequired Gate

Layer / File(s) Summary
SessionHeatCaptureRequired and conditional capture gates
internal/core/runtime/persistence.go, internal/core/runtime/heat_capture_prune_test.go, internal/core/runtime/persistence_test.go
New exported predicate SessionHeatCaptureRequired returns true when HeatColorsEnabled or AutoSortRecentInterval > 0. It gates captureHeatIntoState inside captureLiveSessions and adds an early return to CaptureSessionHeatWithConfig. Tests cover skip, capture, and auto-sort-recent scenarios.
captureLiveSidebarHeat (bool, error) return and daemon loop
internal/app/restore.go, internal/app/restore_test.go
captureLiveSidebarHeat gains a captured bool return and uses SessionHeatCaptureRequired to short-circuit without touching tmux. The daemon loop gates its success continuation on captured. New and updated tests assert the captured boolean for disabled and enabled configurations.

MetadataService Batching, Caching, and Config Reload

Layer / File(s) Summary
tmuxcli.Client.SessionPaths batch method
internal/adapters/tmuxcli/client.go, internal/adapters/tmuxcli/client_test.go
New Client.SessionPaths issues one tmux list-panes -a call and returns a session-to-path map filtering to active window/pane rows. Test verifies single call, active-pane filtering, malformed-row handling, and omission of missing sessions.
MetadataService batch paths and Reconcile caching
internal/app/metadata_service.go, internal/app/metadata_service_test.go
MetadataService gains a Config ports.ConfigPort field and a metadataSessionPathsQuery interface. capture and Reconcile switch to metadataLiveSessionPaths (batch then per-session fallback). Reconcile memoises RepoInfo and WatchTargets per normalised cache key. Tests assert batch usage, partial fallback, batch-failure fallback, and repo-discovery deduplication.
MetadataService.Run config reload and disabled-subline wait
internal/app/metadata_service.go, internal/app/metadata_service_run_config_test.go
Run maintains a current config snapshot and reloads via loadRunConfig each cycle. When the metadata subline is disabled, it waits on ReconcileRequests/timer via waitMetadataReconcile instead of exiting. Tests verify watcher survival on transient reload error, watcher stop on disabled config, and watcher restart on re-enabled config.

Repository Guidance

Layer / File(s) Summary
Agent instructions and architecture map
AGENTS.md
New file documents the repository architecture including module responsibilities, recommended verification commands, safety constraints for runtime/user data, atomic write semantics, dependency boundaries, and expected behavior around missing/malformed state and disabled flags.

Sequence Diagram(s)

sequenceDiagram
  rect rgba(100, 149, 237, 0.5)
    Note over Run: Config reload loop with dynamic enable/disable
  end
  participant Run as MetadataService.Run
  participant loadRunConfig
  participant ConfigPort
  participant waitMetadataReconcile
  participant Reconcile
  participant metadataLiveSessionPaths
  participant SessionPaths as tmuxcli.SessionPaths
  participant SessionPath as tmuxcli.SessionPath
  participant gitCache as git RepoInfo/WatchTargets (memoised)

  loop Each Run iteration
    Run->>loadRunConfig: ctx, fallback snapshot
    loadRunConfig->>ConfigPort: LoadConfig (if Config present)
    ConfigPort-->>loadRunConfig: snapshot or error
    loadRunConfig-->>Run: current snapshot
    
    alt metadata subline enabled
      Run->>Reconcile: ctx, sessions
      Reconcile->>metadataLiveSessionPaths: sessions
      metadataLiveSessionPaths->>SessionPaths: batch lookup
      SessionPaths-->>metadataLiveSessionPaths: partial map
      metadataLiveSessionPaths->>SessionPath: per-session fill missing
      SessionPath-->>metadataLiveSessionPaths: path
      metadataLiveSessionPaths-->>Reconcile: full session→path map
      Reconcile->>gitCache: RepoInfo per normalised key (deduplicated)
      gitCache-->>Reconcile: repo result
      Reconcile->>gitCache: WatchTargets per normalised worktree key (deduplicated)
      gitCache-->>Reconcile: targets result
    else metadata subline disabled
      Run->>waitMetadataReconcile: ctx
      waitMetadataReconcile-->>Run: ReconcileRequests or timer
    end
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • bnema/tmux-session-sidebar#59: Both PRs refactor git ahead/behind/upstream divergence logic in internal/adapters/gitcli/git.go and internal/adapters/gitgo/git.go; this PR builds a comparisonDivergence layer on top of the divergence fast-path wiring introduced in #59.
  • bnema/tmux-session-sidebar#88: Both PRs modify internal/app/runtime_migrate.go and internal/app/session_order.go to change how tmux.json is selected, decoded, and loaded under durable StateDir semantics.
  • bnema/tmux-session-sidebar#89: Both PRs modify internal/app/restore.go and its session heat handling; this PR gates heat capture via SessionHeatCaptureRequired while #89 adds a post-restore transient heat reset for successfully restored sessions.

Poem

🐇 Hoppity-hop through the diff I go,
Divergence helpers lined up in a row,
Atomic renames keep the config files whole,
Batch session paths on a single-pane stroll,
Heat skipped when nothing demands it so —
Cleaner and crisper, watch the green tests glow! ✨

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 6.25% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The PR title 'chore(backlog): complete inbox performance chores' is vague and uses non-descriptive language ('chores', 'inbox', 'backlog') that doesn't clearly convey the specific technical changes made, despite the PR summary describing substantial changes across multiple systems. Consider using a more descriptive title that reflects the main technical accomplishment, such as 'refactor(perf): improve divergence computation and add batched session paths' or grouping by primary area of change.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch chore/backlog-inbox-chores

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/app/runtime_migrate_test.go`:
- Around line 94-181: Add a new test function to cover the regression case where
the current state file in StateDir/tmux.json exists but is malformed and no
candidate state exists for migration. This test should set up a scope with a
valid metadata file, write invalid JSON content to StateDir/tmux.json
(simulating corruption), call ensureRuntimeStateMigratedAndLoad, and assert that
loaded returns false and an error is returned rather than defaulting to an
initialized state. The test should verify that the function properly defers to
canonical error handling instead of returning a reusable state when the current
state is corrupted.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 31344ce4-ba9e-455a-a3eb-3d65682d79e5

📥 Commits

Reviewing files that changed from the base of the PR and between f49e761 and 5fbc5b9.

📒 Files selected for processing (26)
  • AGENTS.md
  • internal/adapters/gitcli/git.go
  • internal/adapters/gitcli/git_test.go
  • internal/adapters/gitgo/git.go
  • internal/adapters/gitgo/git_test.go
  • internal/adapters/storefs/store.go
  • internal/adapters/storefs/store_test.go
  • internal/adapters/tmuxcli/client.go
  • internal/adapters/tmuxcli/client_test.go
  • internal/app/hooks.go
  • internal/app/hooks_atomic.go
  • internal/app/hooks_json.go
  • internal/app/hooks_special.go
  • internal/app/hooks_test.go
  • internal/app/metadata_service.go
  • internal/app/metadata_service_run_config_test.go
  • internal/app/metadata_service_test.go
  • internal/app/restore.go
  • internal/app/restore_test.go
  • internal/app/runtime_migrate.go
  • internal/app/runtime_migrate_test.go
  • internal/app/session_order.go
  • internal/core/persisted/state.go
  • internal/core/runtime/heat_capture_prune_test.go
  • internal/core/runtime/persistence.go
  • internal/core/runtime/persistence_test.go
💤 Files with no reviewable changes (1)
  • internal/core/runtime/persistence_test.go

Comment thread internal/app/runtime_migrate_test.go
@bnema bnema merged commit c58e242 into main Jun 22, 2026
4 checks passed
@bnema bnema deleted the chore/backlog-inbox-chores branch June 22, 2026 17:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant