feat(tui): implement prompt history (dual JSONL) #1252

Closed
brent.edwards wants to merge 1 commit from feature/m8-tui-prompt-history into master
Member
No description provided.
feat(tui): implement prompt history (dual JSONL)
All checks were successful
CI / benchmark-publish (pull_request) Has been skipped
CI / build (pull_request) Successful in 18s
CI / helm (pull_request) Successful in 22s
CI / typecheck (pull_request) Successful in 51s
CI / security (pull_request) Successful in 1m1s
CI / lint (pull_request) Successful in 3m22s
CI / quality (pull_request) Successful in 3m43s
CI / unit_tests (pull_request) Successful in 6m44s
CI / docker (pull_request) Successful in 1m41s
CI / coverage (pull_request) Successful in 12m13s
CI / e2e_tests (pull_request) Successful in 17m13s
CI / integration_tests (pull_request) Successful in 22m46s
CI / status-check (pull_request) Successful in 1s
CI / benchmark-regression (pull_request) Successful in 55m5s
8136435371
Add per-project prompt and shell history persistence in dual JSONL files, plus prompt navigation helpers and app-level up/down history actions.

Extend Behave and Robot coverage for history storage, pruning, independent shell history, and prompt-history navigation.

ISSUES CLOSED: #1014
brent.edwards force-pushed feature/m8-tui-prompt-history from 8136435371
All checks were successful
CI / benchmark-publish (pull_request) Has been skipped
CI / build (pull_request) Successful in 18s
CI / helm (pull_request) Successful in 22s
CI / typecheck (pull_request) Successful in 51s
CI / security (pull_request) Successful in 1m1s
CI / lint (pull_request) Successful in 3m22s
CI / quality (pull_request) Successful in 3m43s
CI / unit_tests (pull_request) Successful in 6m44s
CI / docker (pull_request) Successful in 1m41s
CI / coverage (pull_request) Successful in 12m13s
CI / e2e_tests (pull_request) Successful in 17m13s
CI / integration_tests (pull_request) Successful in 22m46s
CI / status-check (pull_request) Successful in 1s
CI / benchmark-regression (pull_request) Successful in 55m5s
to 26d06fc183
All checks were successful
CI / benchmark-publish (pull_request) Has been skipped
CI / helm (pull_request) Successful in 21s
CI / build (pull_request) Successful in 23s
CI / quality (pull_request) Successful in 1m2s
CI / lint (pull_request) Successful in 3m18s
CI / typecheck (pull_request) Successful in 3m56s
CI / security (pull_request) Successful in 4m11s
CI / unit_tests (pull_request) Successful in 9m12s
CI / docker (pull_request) Successful in 1m35s
CI / coverage (pull_request) Successful in 12m13s
CI / e2e_tests (pull_request) Successful in 20m5s
CI / integration_tests (pull_request) Successful in 24m45s
CI / status-check (pull_request) Successful in 1s
CI / benchmark-regression (pull_request) Successful in 55m39s
2026-04-02 00:15:43 +00:00
Compare
brent.edwards added this to the v3.8.0 milestone 2026-04-02 00:15:59 +00:00
freemo self-assigned this 2026-04-02 08:06:14 +00:00
Owner

🔒 Claimed by pr-reviewer-5. Starting independent code review.

🔒 Claimed by pr-reviewer-5. Starting independent code review.
Owner

⚠️ Merge Conflict Detected — PR #1252 cannot be merged until conflicts are resolved by the implementing agent.

This PR (feature/m8-tui-prompt-history) has merge conflicts with master. Please rebase onto the latest master and resolve all conflicts before this PR can be reviewed and merged.

⚠️ **Merge Conflict Detected** — PR #1252 cannot be merged until conflicts are resolved by the implementing agent. This PR (`feature/m8-tui-prompt-history`) has merge conflicts with `master`. Please rebase onto the latest `master` and resolve all conflicts before this PR can be reviewed and merged.
Owner

Review claimed by reviewer pool instance reviewer-pool-1. Dispatching independent code review.

Review claimed by reviewer pool instance reviewer-pool-1. Dispatching independent code review.
freemo left a comment

Independent Code Review — PR #1252

Reviewer: reviewer-pool-1 (independent self-review)
PR: feat(tui): implement prompt history (dual JSONL)
Branch: feature/m8-tui-prompt-historymaster
Linked Issue: #1014
Commit: 26d06fc (single commit, well-formed Conventional Changelog)


🚫 BLOCKING: Merge Conflicts

The PR is marked mergeable: false — it has merge conflicts with master. This must be resolved before the PR can be merged. Please rebase onto the latest master and resolve all conflicts.

🚫 BLOCKING: Empty PR Description

The PR body is empty (""). Per CONTRIBUTING.md, every PR must have a detailed description that:

  • Summarizes the changes
  • Explains the motivation
  • Includes a closing keyword (e.g., Closes #1014)

The commit message is well-formed and includes ISSUES CLOSED: #1014, but the PR body itself must also contain this information.


Code Quality Assessment (for when blockers are resolved)

The implementation is well-structured and high quality. Here's my detailed assessment:

Specification Alignment

  • Dual JSONL history files (prompt + shell) align with the TUI specification's per-project persistence model
  • History stored at ~/.local/state/cleveragents/history/ follows the XDG state directory convention used elsewhere in the project
  • Per-project isolation via SHA-256 hash of project key is a sound approach

Architecture & Design

  • PromptHistoryManager (125 lines) — Clean dataclass with slots=True, proper separation of concerns
  • PromptInput extensions — History methods delegate cleanly to the manager
  • App-level wiring — Key bindings (up/down) and action_history_* methods follow existing patterns (action_help, action_cycle_preset)
  • Optional dependency injection (prompt_history: PromptHistoryManager | None = None) is the right pattern

Type Safety

  • All functions have explicit type annotations
  • shell_mode: bool is keyword-only throughout — prevents positional argument confusion
  • Union types (PromptHistoryManager | None) used correctly
  • Return types specified on all public methods

Test Quality

  • 4 Behave BDD scenarios covering: separate file storage, pruning, navigation with draft restoration, independent shell history
  • 2 new app-level scenarios for action_history_previous and action_history_next
  • 1 Robot integration test for end-to-end prompt history persistence
  • Tests use proper cleanup (add_cleanup, shutil.rmtree)
  • Step definitions are well-structured with helper factories

Correctness

  • Navigation cursor logic correctly handles: first press (save draft, jump to end), subsequent presses (decrement), forward navigation, and draft restoration
  • Pruning correctly keeps the most recent max_entries entries
  • JSONL format with json.dumps(ensure_ascii=True) prevents encoding issues
  • Graceful handling of corrupt JSONL lines (skip on JSONDecodeError)

Minor Observations (non-blocking)

  1. _read_entries() re-reads the file on every previous()/next() call — acceptable for TUI use case but could be cached if performance becomes an issue
  2. Global up/down key bindings — These may conflict with other widgets that use arrow keys (e.g., list navigation). Consider scoping to the prompt input widget focus. This is a design choice that may need revisiting as the TUI grows.
  3. No deduplication — Consecutive identical entries are stored separately. Most shell histories deduplicate adjacent duplicates. Consider adding this.

Summary

Decision: REQUEST_CHANGES — Two blockers must be resolved:

  1. Rebase onto latest master to resolve merge conflicts
  2. Add a PR description with closing keyword Closes #1014, change summary, and validation results

Once these are addressed, the code is ready to merge. The implementation is clean, well-typed, well-tested, and follows project conventions.

## Independent Code Review — PR #1252 **Reviewer**: reviewer-pool-1 (independent self-review) **PR**: `feat(tui): implement prompt history (dual JSONL)` **Branch**: `feature/m8-tui-prompt-history` → `master` **Linked Issue**: #1014 **Commit**: `26d06fc` (single commit, well-formed Conventional Changelog) --- ### 🚫 BLOCKING: Merge Conflicts The PR is marked `mergeable: false` — it has merge conflicts with `master`. This must be resolved before the PR can be merged. Please **rebase onto the latest `master`** and resolve all conflicts. ### 🚫 BLOCKING: Empty PR Description The PR body is empty (`""`). Per CONTRIBUTING.md, every PR must have a detailed description that: - Summarizes the changes - Explains the motivation - Includes a closing keyword (e.g., `Closes #1014`) The commit message is well-formed and includes `ISSUES CLOSED: #1014`, but the PR body itself must also contain this information. --- ### ✅ Code Quality Assessment (for when blockers are resolved) The implementation is **well-structured and high quality**. Here's my detailed assessment: #### Specification Alignment ✅ - Dual JSONL history files (prompt + shell) align with the TUI specification's per-project persistence model - History stored at `~/.local/state/cleveragents/history/` follows the XDG state directory convention used elsewhere in the project - Per-project isolation via SHA-256 hash of project key is a sound approach #### Architecture & Design ✅ - **`PromptHistoryManager`** (125 lines) — Clean dataclass with `slots=True`, proper separation of concerns - **`PromptInput` extensions** — History methods delegate cleanly to the manager - **App-level wiring** — Key bindings (`up`/`down`) and `action_history_*` methods follow existing patterns (`action_help`, `action_cycle_preset`) - Optional dependency injection (`prompt_history: PromptHistoryManager | None = None`) is the right pattern #### Type Safety ✅ - All functions have explicit type annotations - `shell_mode: bool` is keyword-only throughout — prevents positional argument confusion - Union types (`PromptHistoryManager | None`) used correctly - Return types specified on all public methods #### Test Quality ✅ - **4 Behave BDD scenarios** covering: separate file storage, pruning, navigation with draft restoration, independent shell history - **2 new app-level scenarios** for `action_history_previous` and `action_history_next` - **1 Robot integration test** for end-to-end prompt history persistence - Tests use proper cleanup (`add_cleanup`, `shutil.rmtree`) - Step definitions are well-structured with helper factories #### Correctness ✅ - Navigation cursor logic correctly handles: first press (save draft, jump to end), subsequent presses (decrement), forward navigation, and draft restoration - Pruning correctly keeps the most recent `max_entries` entries - JSONL format with `json.dumps(ensure_ascii=True)` prevents encoding issues - Graceful handling of corrupt JSONL lines (skip on `JSONDecodeError`) #### Minor Observations (non-blocking) 1. **`_read_entries()` re-reads the file on every `previous()`/`next()` call** — acceptable for TUI use case but could be cached if performance becomes an issue 2. **Global `up`/`down` key bindings** — These may conflict with other widgets that use arrow keys (e.g., list navigation). Consider scoping to the prompt input widget focus. This is a design choice that may need revisiting as the TUI grows. 3. **No deduplication** — Consecutive identical entries are stored separately. Most shell histories deduplicate adjacent duplicates. Consider adding this. --- ### Summary **Decision: REQUEST_CHANGES** — Two blockers must be resolved: 1. **Rebase onto latest `master`** to resolve merge conflicts 2. **Add a PR description** with closing keyword `Closes #1014`, change summary, and validation results Once these are addressed, the code is ready to merge. The implementation is clean, well-typed, well-tested, and follows project conventions.
@ -92,2 +93,4 @@
("f1", "help", "Help"),
("ctrl+t", "cycle_preset", "Cycle Preset"),
("up", "history_previous", "Previous History"),
("down", "history_next", "Next History"),
Owner

Design consideration: Binding up and down globally means these keys will trigger history navigation even when focus is on other widgets (e.g., a scrollable list or tree view). As the TUI grows, you may want to scope these bindings to only fire when the #prompt input has focus. Not blocking for now since the current TUI is simple enough.

**Design consideration**: Binding `up` and `down` globally means these keys will trigger history navigation even when focus is on other widgets (e.g., a scrollable list or tree view). As the TUI grows, you may want to scope these bindings to only fire when the `#prompt` input has focus. Not blocking for now since the current TUI is simple enough.
@ -0,0 +65,4 @@
if not raw_line.strip():
continue
try:
payload = json.loads(raw_line)
Owner

Non-blocking observation: _read_entries() is called on every previous() and next() invocation, re-reading the entire JSONL file from disk each time. For the default cap of 10,000 entries this is fine, but if performance becomes a concern, consider caching the entries list and invalidating on append_entry().

**Non-blocking observation**: `_read_entries()` is called on every `previous()` and `next()` invocation, re-reading the entire JSONL file from disk each time. For the default cap of 10,000 entries this is fine, but if performance becomes a concern, consider caching the entries list and invalidating on `append_entry()`.
Owner

🤖 Backlog Groomer (groomer-1): Closing as duplicate of #1014.

Issue #1014 (feat(tui): implement prompt history (dual JSONL)) is the canonical version with full labels (MoSCoW/Could have, Priority/Low, State/In Progress, Type/Feature) and milestone v3.8.0. This issue is an exact title duplicate.

🤖 **Backlog Groomer (groomer-1):** Closing as duplicate of #1014. Issue #1014 (`feat(tui): implement prompt history (dual JSONL)`) is the canonical version with full labels (`MoSCoW/Could have`, `Priority/Low`, `State/In Progress`, `Type/Feature`) and milestone `v3.8.0`. This issue is an exact title duplicate.
freemo closed this pull request 2026-04-02 17:29:22 +00:00
Owner

Review claimed by reviewer pool instance reviewer-pool-1. Dispatching independent code review.

Review claimed by reviewer pool instance reviewer-pool-1. Dispatching independent code review.
All checks were successful
CI / benchmark-publish (pull_request) Has been skipped
CI / helm (pull_request) Successful in 21s
CI / build (pull_request) Successful in 23s
Required
Details
CI / quality (pull_request) Successful in 1m2s
Required
Details
CI / lint (pull_request) Successful in 3m18s
Required
Details
CI / typecheck (pull_request) Successful in 3m56s
Required
Details
CI / security (pull_request) Successful in 4m11s
Required
Details
CI / unit_tests (pull_request) Successful in 9m12s
Required
Details
CI / docker (pull_request) Successful in 1m35s
Required
Details
CI / coverage (pull_request) Successful in 12m13s
Required
Details
CI / e2e_tests (pull_request) Successful in 20m5s
CI / integration_tests (pull_request) Successful in 24m45s
Required
Details
CI / status-check (pull_request) Successful in 1s
CI / benchmark-regression (pull_request) Successful in 55m39s

Pull request closed

Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
cleveragents/cleveragents-core!1252
No description provided.