feat: integrate history slices 1-6 (review fixes) and selection correctness - #1457
Closed
carolitascl wants to merge 69 commits into
Closed
carolitascl wants to merge 69 commits into
carolitascl wants to merge 69 commits into
Conversation
Slice 1/6 of the PR Gentleman-Programming#819 split (maintainer-requested review slices). - atomic-write: same-dir tmp+rename JSON writer, concurrent-instance-safe staging names, never throws - store: project identity (realpath+sha256[:16], raw-path fallback, 24-char collision re-key), project/seed/global/registry path derivations, advisory registry with fail-open reads and atomic writes, tolerant JSONL line parser, lazy per-instance session writer with command filtering - extension entry: identity constants and capture-only wiring (before_agent_start -> appendSessionCapture); migration, seeding, selector, deletion, and GC join in later slices - tests: 32 node:test cases covering storage concurrency and recovery (parallel writers, interleaved captures, burst order integrity, torn-line matrix + crash-tail recovery window, rapid same-target atomic writes with zero staging residue, two-instance registry interleaving, collision re-key, corrupt/wrong-shape fail-open) - test vectors are machine-independent: literal cwds exercise the documented raw-string fallback identically on every platform Gates: scoped history tests 32/32 green. verify-package-files and package-manifest failures are pre-existing environmental (gitignored contracts/.DS_Store; missing node_modules) and reproduce on vanilla origin/main.
Review fix (CodeRabbit #5160388228): ensureRegistryEntry now searches the registry for an existing mapping of the incoming cwd before the collision branch, returning the existing short or long key unchanged. Previously, re-entering a cwd that an earlier collision had re-keyed to 24 chars re-triggered the collision and flipped the other occupant's key every time — collision assignments were not stable. Adds a stability test: the re-keyed cwd keeps its long key, the short-hash holder keeps its key, and the registry bytes do not change across re-entries. Note: the atomic-write staging-name race CodeRabbit reported in the original commit was already hardened on this branch (unique .tmp-<pid>-<ts> staging + unlink-on-failure); no further change.
…y APIs Slice 2/6 of the PR Gentleman-Programming#819 split (maintainer-requested review slices). - store: reader/query section — file listing with mtime resolution, drain ordering (newest entry ts, mtime fallback; stable under atomic rewrites), dedup + tombstone filter + cap drain, project scope drain (hash dir) and global scope drain (all project dirs, legacy global seed last); dead generator fileEntriesBackward (zero callers) dropped - hide-prompts: tombstone file contract (fail-open reader, atomic sorted writer, shared dedup key) — lands here because the drain APIs filter hidden prompts via the optional stateDir parameter; slice 5 delivers deletion semantics on top - selector-helpers (new): entry/dedup-key normalization, keep-first read-time dedup, records shaping with provenance, result filter with MAX_RESULTS cap; windowing/nav helpers follow in slice 3 - tests: 21 new node:test cases (cumulative 53/53): drain ordering across mixed mtimes, hidden-prompt filtering incl. corrupt hidden.json fail-open, dedup key normalization, cap at exactly 10000, hide/write contract incl. ENOTDIR failure; portable CWD literals throughout (no machine-specific paths) Gates: cumulative scoped history tests 53/53 green (slice-1 set unchanged). Known pre-existing environmental gate failures unchanged (contracts/.DS_Store; missing node_modules for package-manifest).
Review fix (Copilot suppressed comment, store.ts): the docblock claimed the legacy global seed is the "newest single source", but the code deliberately appends it after sorting (`// legacy last`) so per-project entries win recency and keep-first dedup. Document the actual, intended behavior instead of changing it: migrated legacy history is the least specific source.
Slice 3/6 of the PR Gentleman-Programming#819 split (maintainer-requested review slices). - selector-helpers: windowing/navigation subset — clamp/visible-range math, move/page selection, lazy-window growth (initial batch, grow triggers, target loading, query full-snapshot), visible-record projection, expanded-history globals hook - index.ts: PromptHistorySelector TUI (fixed-row layout, centered preview pane, search filter, Tab project/global scope toggle, grow-before-move navigation, PgDn catch-up, End full jump, wheel handling over fixed 30-row geometry, width-change pre-clamp), overlay glue (bottom-center anchored ctx.ui.custom factory), drainForScope + recordsFromEntries, wiring for ctrl+shift+r shortcut, history command, and tool_call overlay dismissal - upstream dead code dropped: notifyIndexProgress/activeIndexProgress sink pair (never fired) and unused fs import - deletion is slice 5: no deleteCurrent, no delete dispatch entry, no delete affordance in the footer hint yet - getWriter still performs no migration/seed bootstrap (slice 4); the selector drains live stores only - tests: 57 new node:test cases (cumulative 110/110): windowing math, lazy window growth contracts, preview layout, 11-entry dispatch table, wheel routing, expanded globals, shortcut/command registration surface, open-close flow with fake ctx; superseded slice-1 registration pin updated to the slice-3 wiring surface Gates: cumulative scoped history tests 110/110 green. esbuild bundle parse of the full extension graph clean. Known pre-existing environmental gate failures unchanged.
…omments Review fixes (Copilot + CodeRabbit on PR Gentleman-Programming#819): - sanitizeForDisplay: astral code points (> 0xFFFF) are re-appended via String.fromCodePoint instead of only the high surrogate at text[i]; emoji and other non-BMP characters no longer lose half their code point in list rows and previews. The low-surrogate skip is retained. - FixedRowText.render: the full-width pad now measures the VISIBLE width (SGR escape sequences stripped), matching the centered branch's measurement; colored list rows previously padded short and could leave ghost characters on overlay dismiss. - Lazy-windowing comment corrected: PRELOAD_BUFFER is 3 (fired in the final 3 loaded rows), not 2 as the stale comment claimed. - Regression pins added for both behavior fixes.
Slice 4/6 of the PR Gentleman-Programming#819 split (maintainer-requested review slices). - load-shared-history (new): v1 shared-history reader, fail-open entry normalization - session-scan (new): transcript JSONL extraction — line-1 admission gate (type/version), text-block extraction, timestamp fallback chain (message-ms -> entry-ISO -> header-ISO -> mtime), prompt length and whitespace rules, one-level encoded-cwd sessions-root scan - store: legacy migration (v1 editor-history.jsonl + pre-v1 editor-history.json -> history-global.jsonl, one-time gate, .imported renames, corrupt-source skip) and project seed bootstrap (transcript scan -> seed.jsonl written ONCE so deletion cannot resurrect, tombstone suppression, cwd matching via encoded sessions dirs) - index.ts: getWriter now runs the upstream init sequence — migrate -> registry -> seed bootstrap -> open instance writer - indexing scope note: upstream's session indexing (session-index.ts + merge-history.ts, ~400 lines) is dead code on the PR branch — zero importers after the store-only drain pivot — and is intentionally absent from this chain (preserved out-of-tree for reference) - tests: 38 new node:test cases (cumulative 148/148): migration one-time gate + .imported renames + corrupt-source skip, seed written-once anti-resurrection + tombstone suppression + regen idempotence, transcript extraction matrix (583-line dev suite preserved), directory scan; tmpdir + fake-cwd fixtures throughout Gates: cumulative scoped history tests 148/148 green. Known pre-existing environmental gate failures unchanged.
…he first-prompt path Review fixes (CodeRabbit on PR Gentleman-Programming#819): - migrateLegacyStores: legacy sources are renamed .imported only AFTER the global seed write succeeds. Previously each source was renamed immediately after reading, so a seed-write failure stranded the collected entries in .imported files with the one-shot seed gate blocking retry — silent data loss. Failure-path test added: a read-only store root makes the seed write throw, sources stay in place, and the retried migration completes and archives them. - promptHistoryExtension: writer init (migrate + registry + seed bootstrap) is scheduled once via setImmediate so the transcript scan never runs on the first-prompt path; prompts arriving before the scheduled init fall back to getWriter()'s synchronous lazy init, whose writerState guard keeps the work single-shot. - Source pin added for the setImmediate scheduling and the retained synchronous fallback.
Slice 6/6 of the PR Gentleman-Programming#819 split (maintainer-requested review slices). - store: GC/compaction section — thresholds (50 files / 5000 lines / keep-newest-10), gcProjectDir entry point, compactFiles (merge all but newest 10 into compact-<ts>.jsonl, atomic write BEFORE originals removed, rm failure tolerated); dead compactProjectDir export (zero callers) dropped — store.ts now carries upstream content minus the documented dead exports - index.ts: session_shutdown handler wired to gcProjectDir (final registration surface: before_agent_start, session_shutdown, tool_call, ctrl+shift+r shortcut, history command) - tests: 9 new node:test cases (cumulative 174/174): threshold no-op below limits, keep-newest-10 untouched, line-threshold trigger, missing-dir and unreadable-file skips, atomic-before-rm ordering, rm-failure tolerance, concurrent append during compaction never loses post-compaction writes; tmpdir fixtures with machine-independent literals throughout Gates: cumulative scoped history tests 174/174 green — the complete six-slice chain. Known pre-existing environmental gate failures unchanged (gitignored contracts/.DS_Store; package-manifest needs node_modules, now installed).
Slice 5/6 of the PR Gentleman-Programming#819 split (maintainer-requested review slices). - store: scope-delete section — sweepFiles (atomic rewrite per affected file, emptied session files kept so live writers stay functional, never fatal), deleteFromProject, deleteFromGlobal - selector-helpers: deletionActionsFor (pure provenance->actions planner) and loadedCountAfterDelete (backfill window math), completing the helper surface - index.ts: deleteCurrent on the selector exactly as upstream — provenance- planned sweep + tombstone ALWAYS written (seed/transcript-sourced entries cannot resurface; the seed is write-once) + splice + backfill + failure toast; ctrl+shift+backspace dispatch entry and footer affordance restored (dispatch table back to 12 entries) - privacy semantics (enforced by tests): hidden.json fail-open read, tombstone precedes any visibility change, deletion from the global seed and project stores is permanent because the seed is written once - tests: 17 new/updated node:test cases (cumulative 165/165): sweep with a concurrent live writer, emptied-file-kept, chmod-000 partial failure toast path, tombstone-always planner pin, unknown-prompt no-op, backfill bounds, dispatch/wheel table pins restored to 12 Gates: cumulative scoped history tests 165/165 green. Known pre-existing environmental gate failures unchanged.
Review fixes (CodeRabbit + Copilot on PR Gentleman-Programming#819): - compactFiles: the compact artifact name now carries process.pid (compact-<pid>-<ts>.jsonl), matching the uniqueness convention of the staging name — two concurrent instances can never target the same compact filename. Filename pin updated accordingly. - session_shutdown comment corrected: compaction runs at the GC thresholds (50 files / 5000 lines / keep-newest-10), not a "1000-line limit" as the stale comment claimed.
- overlay confines to the editor column while the gentle-shell fullscreen sidebar paints (pi-tui margin resolved live via the visible() hook; rail 50 + gap 3 + 1 padding) - responsive picker header: inline / stacked (tablet) / compact (mobile) modes with fit-driven thresholds and an abbreviated scope radio; overlay stays a fixed 30-row grid in every mode - selector always opens on empty stores; registry collision re-key guard; biome-clean formatting across the module - tests: +overlay-margin, +header-layout; history suite green under the node runner (188 pass)
Sync of pi-history a1c13b9: headerCountsText() now serves the inline and stacked header branches; no behavior change.
- import ExtensionCommandContext (the real pi-coding-agent export) instead of the shim-only ShortcutContext name; ctx params take Pick<ExtensionCommandContext, "ui"> - skipIf shim casts its callback to TestFn's return type so node's test options overload typechecks (6 sealed-file test files)
- replace hardcoded /Users/admin/Dev/pi/pi-history and /Users/admin/Dev/github/pi constants with synthetic /pi-history-fixtures/project-a|project-b literals - pin projectHash with a portable known vector (nonexistent path falls back to raw-string hashing), replacing the layout-dependent 28e0f06819c468cb digest - registry tests use mkdtemp project fixtures with relative assertions - update the 6 session-slug literals coupled to the old cwd constant - replace bun:test-only test.skipIf wrappers with node:test-compatible skip-option wrappers in gc/scope-delete/drain-order/seed-bootstrap - bring gc/scope-delete/drain-order/seed-bootstrap in line with the newer reviewed pi-history test versions (drift since the slices were cut) Verified: node --test tests/history-*.test.ts 188 pass / 0 fail Review: review-134310abd04cf6c9 (reliability lens, approved)
The test.skipIf-style wrappers typed their callback as () => unknown, which is not assignable to node:test's TestFn (return void | Promise<void>). Type the callback accordingly to clear the 4 new TS2345 diagnostics reported by the type gate.
Sync 72b5adf faithfully mirrored pi-history main, which never received the review fixes from the slice reviews (PR Gentleman-Programming#819); the sync silently reverted them along with their test pins. Restore everything on the slice-06 PR branch, adapted to the current upstream-parity sources, with the test-hygiene commits (portable fixtures, skip wrappers) already cherry-picked in: - store: compact artifact name is pid-scoped (compact-<pid>-<ts>.jsonl) so two concurrent instances can never rename onto the same file (silent compact loss) - store: migrateLegacyStores renames legacy sources to .imported only AFTER the global seed write succeeds - a failed write no longer strands entries with the one-shot gate blocking retry - store: ensureRegistryEntry returns an existing long-key mapping unchanged so collision assignments stay stable across calls - index: sanitizeForDisplay re-appends astral code points whole (String.fromCodePoint) - emoji no longer lose half their code point - index: FixedRowText.render pads by the SGR-stripped visible width - colored rows no longer fall short and leave ghost characters - index: writer init (migrate/registry/seed) is scheduled via setImmediate so bootstrap never runs on the first-prompt path - index: PRELOAD_BUFFER comment corrected to the constant's real value - tests: restore the GC crash-safety trio (atomic ordering, rm-failure tolerance, active-writer mid-compaction) plus the pid filename pin, the migration retry test, the registry stability test, the setImmediate scheduling pin, and the padding + astral pins - compactProjectDir stays exported for upstream parity (no knip gate configured); gcProjectDir remains the wired and tested entry point Gates: full history suite 196 pass / 0 fail under bun (node:test sources). No tsc/typecheck script exists in this repo; the suite run parses every changed file via node's type stripping.
extensions/history/index.ts imported `ShortcutContext`, a type that only exists in the dev repo's @types shim — the real @earendil-works/pi-coding-agent exports `ExtensionCommandContext`, so the type gate added to main reports TS2305 on this branch's CI merge. Import `ExtensionCommandContext` and narrow both handler contexts to `Pick<ExtensionCommandContext, "ui">` (the only member they use), mirroring the fix already carried on the slice-6 branch.
extensions/history/index.ts imported `ShortcutContext`, a type that only exists in the dev repo's @types shim — the real @earendil-works/pi-coding-agent exports `ExtensionCommandContext`, so the type gate added to main reports TS2305 on this branch's CI merge. Import `ExtensionCommandContext` and narrow both handler contexts to `Pick<ExtensionCommandContext, "ui">` (the only member they use), mirroring the fix already carried on the slice-6 branch.
extensions/history/index.ts imported `ShortcutContext`, a type that only exists in the dev repo's @types shim — the real @earendil-works/pi-coding-agent exports `ExtensionCommandContext`, so the type gate added to main reports TS2305 on this branch's CI merge. Import `ExtensionCommandContext` and narrow both handler contexts to `Pick<ExtensionCommandContext, "ui">` (the only member they use), mirroring the fix already carried on the slice-6 branch.
shift+end anchored selection, alt+a select-all, and replace-on-key (backspace / delete / printable) with reverse-video highlight on the content rows and the "N chars selected" hint on the petal's bottom rule. The engine (lib/selection-engine.ts) drives the editor's own internals through the EditorInternals surface and degrades to pure passthrough if that surface drifts on a pi upgrade — selection features off, never a crash. handleInput dispatches selection keys in front of the petal's chain (Esc gates, idle-clear, autocomplete all preserved via handleInputNative). With the feature native to the petal, the pi-select-del factory composition (and the cross-extension focus handoff it required) becomes unnecessary for gentle setups. Tests: 6 new cases driving a real CustomEditor and the framed petal prompt end to end (selection, replace, collapse, zero-width no-op, highlight + hint, degraded passthrough). Gates: typecheck clean (no regressions), targeted suite 296 pass / 0 fail.
…engine The packed runtime resolves deep pi-tui subpath imports by joining onto the resolved entry file, so `@earendil-works/pi-tui/dist/keys.js` loaded as `dist/index.js/dist/keys.js` and broke every extension load that reaches lib/selection-engine.ts (CI: asset-installation-runtime.test.ts expected zero extension errors). Root package imports are proven safe in that loader. decodeKittyPrintable is exported from the pi-tui package root, so only the modifyOtherKeys half of decodePrintableKey is vendored verbatim into lib/pi-tui-keys.ts (parse + decode, same regexes and modifier masks), with decodePrintableKey composing root decodeKittyPrintable over the vendored half. Behavior is identical to pi-tui dist/keys.js. Tests: asset-installation-runtime.test.ts 1 pass / 0 fail under node --experimental-strip-types --test; selection-engine suite 6 pass / 0 fail; npm run typecheck clean with no regressions.
…ndo replacement The extension-era suites were replaced by the native engine port, which left the review-requested terminal-key and undo contracts untested. Add focused node:test cases alongside the existing six: - kitty CSI-u press replaces the selection; a flag-2 release is dropped before any key matching and keeps the selection active - repeated shift+home at the line edge keeps the selection (held-key auto-repeat must not collapse the span) - modifyOtherKeys printable decodes; ctrl-modified and control codepoints are rejected by the vendored decoder - CSI-u-encoded DEL (127) and C1 (0x9b) codepoints replace the selection with a pure delete: the splice guard strips the control character so it is never inserted - printable replacement pushes exactly one undo snapshot before the splice, so replacement reverts in a single undo step Gates: node --test selection suite 11 pass / 0 fail; check-types baseline gate pass (no regressions).
incremental re-render, and the fullscreen interaction's stale cell state survived it: keys kept working (selection, delete, everything dispatched and matched at the byte level) while the screen no longer updated. Field-reported as "selection keys stop working after gentle commands", with /reload as the only recovery; PID-stamped stdin taps proved the keypresses were delivered and matched while the screen stayed stale. Add lib/overlay-repaint.ts withOverlayRepaint: wraps an overlay's done callback so the close path runs done() first, then tui.invalidate() + tui.requestRender() to repaint every cell. Paint failures are swallowed; done failures propagate. Wired into the four gentle overlays: agents view, command palette, usage view, changes file chooser. Tests: 4 new cases (order done->invalidate->requestRender, null close, swallowed paint failure, propagating done failure). Gates: typecheck clean (no regressions), targeted suite 290 pass / 0 fail run with EDITOR/VISUAL neutralized per AGENTS.md after editor-launch leaks.
on a long session rebuilt the whole transcript synchronously - a ~5s freeze that exactly matched the reported overlay close delay. Use tui.requestRender(true) instead: resetRenderState clears the written-frame buffer so the next paint rewrites every cell (clearing the stale overlay ghost) while component row caches stay warm.
# Conflicts: # extensions/gentle-shell.ts
Undo the merge of fix/overlay-close-repaint so PR Gentleman-Programming#1402 stays selection-focused as requested in review: the overlay-close repaint fix moves to its own small PR. The tree after this revert is byte-identical to cd94577 (4 files, +695/-2 vs main: selection engine, vendored decoder, petal wiring, tests). The overlay fix itself is unchanged and lands separately from fix/overlay-close-repaint, split out of Gentleman-Programming#806.
Review follow-up on the slice-01 PR: the before_agent_start handler recorded delivered prompts by default while the deletion UI is still unshipped, so an intermediate release could accumulate sensitive prompts with no removal path. - Capture is now strictly opt-in via GENTLE_PI_HISTORY_CAPTURE=1|true|on (default off); the switch doubles as the disable path, is checked per prompt, and a disabled session writes nothing - no registry entry, no files. - promptHistoryExtension takes injectable deps (env/root/cwd/ instanceId/now) with one writer closure per extension load. - New tests: strict opt-in matrix, default-off inertness, opted-in capture, disable-leaves-existing-files. - docs/prompt-history.md documents the switch, storage locations, permissions/readers, and disable/removal semantics; the README docs table gains a pointer.
Resolves PR Gentleman-Programming#1390's README.md conflict: main's 3.5 documentation restructure replaced the former docs table; the prompt-history row is re-applied in the new Destination/Purpose shape. No other conflicts; all other upstream changes auto-merged.
A corrupt or unreadable hide file previously loaded as an empty hidden set (fail open), resurfacing prompts the user may have hidden because they contain secrets. The next hide also rewrote the file clean, silently clearing the incident. - readHiddenPrompts replaces loadHiddenPrompts: ENOENT stays trusted-empty (nothing ever hidden); any other read error, JSON parse failure, or non-array shape is untrusted (unreadable/corrupt/ malformed) and carries a recovery message naming hidden.json - hidePrompt refuses to write over an untrusted file: recovery is the explicit delete-or-restore of hidden.json, never a silent rewrite - drainProject/drainGlobal return DrainResult: untrusted tombstones block the drain (status "blocked", no prompts field) so the future selector UI must surface the warning; no stateDir keeps raw drain semantics Tests: rewrite T26 to pin the refusal + byte-unchanged file + manual unlink recovery; add malformed-shape, junk-item tolerance, and chmod 000 unreadable cases; drains pin the blocked shape (no prompts field) and the missing-file-stays-ok case.
review-repository-windows failed with CandidateViewError "candidate view owner preparation failed (ETIMEDOUT)" during worktree preparation, while test/verify/session-transport all passed. No code change; re-running the checks via an empty commit because workflow rerun requires upstream admin rights.
…selector # Conflicts: # extensions/history/index.ts # tests/history-session-writer.test.ts
…-04-seed # Conflicts: # extensions/history/index.ts # extensions/history/store.ts # tests/history-session-writer.test.ts
…delete # Conflicts: # extensions/history/index.ts
…6-gc # Conflicts: # extensions/history/index.ts # tests/history-session-writer.test.ts
Review follow-up on the slice-04 PR: the selector read path (openHistorySelector -> drainForScope -> getWriter) ran legacy migration and seed bootstrap without checking the capture preference, so opening /history with capture disabled silently imported past prompts into searchable store files. - openHistorySelector warns and returns before any drain unless GENTLE_PI_HISTORY_CAPTURE=1|true|on; drainForScope adds a defense-in-depth early return. The warm-up and capture handler were already gated; the read path now matches. - Define the previously-undefined AGENT_DIR constant: migrateLegacyStores had been dead code (swallowed ReferenceError) since the deps refactor. - docs/prompt-history.md: "Legacy migration and seeding are opt-in" - imports create new searchable copies under ~/.pi/agent/history, source transcripts stay untouched, disabling does not remove imported copies. - tests/history-off-path.test.ts: with capture off, extension load writes nothing and the history command imports nothing and warns.
Review follow-up on the slice-05 PR (plus restoration of a fix clobbered by the Sept-24 merge train): - Restore the fail-closed tombstone contract from slice-2: hidden.json reads return trusted (missing/valid array) or untrusted (unreadable/ corrupt/malformed) with a recovery message naming the file; hidePrompt refuses to silently rewrite an untrusted file; drains return a blocked DrainResult with no prompts field; seed bootstrap fails closed on untrusted tombstones. - Two-step delete confirmation: the first ctrl+shift+backspace arms the selected row with scope-aware copy, the second executes, any other key disarms. The copy distinguishes deleting a stored prompt (physical store removal + tombstone) from hiding a session-derived prompt (tombstone only; transcripts are immutable). - Failure semantics made explicit: store-delete failures toast and abort before any tombstone write; hide failures toast distinctly (session path aborts; editor path reports the store row was removed while the hide failed). - docs/prompt-history.md: "Delete vs hide" section covering provenance semantics, failure behavior, and corrupt-hidden.json recovery. - Tests: restore the fail-closed hide/drain suites, adapt drain-order to the DrainResult contract, and add history-delete-confirm covering arming, provenance, and failure paths.
Port slice-5's restoration (89ac348) of the fail-closed tombstone contract onto slice-4 so PR Gentleman-Programming#1391 does not reintroduce the fail-open hidden.json behavior after Gentleman-Programming#1392 merges: - hide-prompts.ts byte-identical to the restored version: readHiddenPrompts returns trusted (missing/valid array) or untrusted (unreadable/corrupt/malformed) with a recovery message naming hidden.json; hidePrompt refuses to rewrite an untrusted file. - store.ts: drains return DrainResult (blocked status carries no prompts field) and bootstrapProjectSeed skips seeding on untrusted tombstones. - index.ts: drainForScope unwraps DrainResult; the selector surfaces the blocked recovery message instead of silently showing entries. - Tests: hide-prompts, drain-hidden, and drain-order suites ported byte-identically from the restored versions. Delete-flow code remains slice-5 scope; nothing delete-related entered this port.
…6-gc # Conflicts: # extensions/history/index.ts # extensions/history/store.ts # tests/history-drain-order.test.ts # tests/history-hide-prompts.test.ts
Review follow-up on the slice-06 PR: the branch mixed the GC/lifecycle rewrite with unrelated selector header-layout, overlay-margin, and scope-radio changes, so reviewers could not assess the destructive store rewrite as one bounded unit. - Remove the layout batch: planHeaderLayout/HeaderLayoutMode, editorOverlayMargin, SIDEBAR_RAIL_OVERLAY_MARGIN/SIDEBAR_OVERLAY_PADDING, SCOPE_RADIO_* constants, and scopeRadioText from selector-helpers; drop their index.ts usage (header mode fields, OptionalRow, headerCountsText, listWheelFirstRow) and delete the header-layout and overlay-margin test files. - Restore preview-layout and wheel-mouse suites byte-exact to slice-5 and revert the constructor child-count pins the layout batch introduced. - Keep the GC/lifecycle work intact: gcProjectDir/compactProjectFile(s), thresholds, the session_shutdown GC hook, and the gc test suite; keep all slice-5 delete-confirm behavior and the fail-closed tombstone contract. - Relocate a merge-misplaced doc comment above openHistorySelector. The layout/formatting work moves to a follow-up PR so the destructive GC rewrite and its failure tests are reviewed as one unit.
Clarify in the prompt-history docs that compaction is housekeeping for performance: it consolidates capture files and drops the oldest entries to bound file/line counts, but it does not remove prompts from the resulting store and is not a data-retention or automatic-deletion policy. Prompts leave the store only through the delete flow (or manual removal), and compaction honors tombstones so deleted prompts stay deleted.
First of three review units for the selector slice (PR Gentleman-Programming#1395 review asked to split it into command/open flow, search/list, and preview/mouse): - /history command and ctrl+shift+r shortcut share one gated entry point: with capture disabled it warns naming GENTLE_PI_HISTORY_CAPTURE and never touches migration, seed, registry, or store files; DrainResult drains unwrap with the fail-closed blocked path surfacing the recovery message instead of any prompts. - Minimal PromptHistorySelector overlay: frame, wrapped-cursor list, esc/enter lifecycle, and the original empty-store policy (notify and return). - selector-helpers: visible-range helpers (moveSelectedIndex, computeVisibleRange, getVisiblePromptRecords, VisibleRange types). - Tests: command-registration (shared entry point, empty-store guard, capture-gate ordering, blocked-drain handling) and openflow-integration adapted to the DrainResult contract.
Second of three review units for the selector slice (PR Gentleman-Programming#1395 review): - Search panel: header, hint row, Input with onSubmit/onEscape, forwardToSearch key fallthrough, and filterPrompts applied over the loaded snapshot via loadedCountForQuery. - Lazy windowing: initial batch, grow-before-move prefetch, PgUp/PgDn catch-up, and Home/End jumps (initialLoadedCount, shouldGrowWindow, nextLoadedCount, loadedCountForTarget, clampSelectedIndex, pageSelectedIndex). - Scope toggle between project and global drains over the fail-closed DrainResult contract: blocked drains revert the scope flip and surface the recovery message. Header gains the loaded segment and the scope radio; the overlay runs under withExpandedHistoryGlobals. - Full dispatch table: up/down/pageUp/pageDown/confirm/tab/cancel/ home/end. - Tests: dispatch, lazy-windowing, selector-windowing, and expanded-globals suites ported/adapted to the current contracts.
Third and final review unit for the selector slice (PR Gentleman-Programming#1395 review): - Preview panel: PREVIEW_ROWS viewport, word-wrapped prompt text with SGR-safe padding, range label, preview scroll with ctrl+shift+up/down completing the 11-entry dispatch table, and offset resets on every list navigation. - Mouse: wheel-only handling with consumed-event routing and region constants (list 5-14, preview 17-26), sign-clamped list wheel through moveDown/moveUp and one-clamped-line preview wheel. - Completed 30-row overlay geometry; overlay glue: selectorTui capture, activeOverlayClose on tool_call, and the post-paste render flush. - Tests: preview-layout and wheel-mouse suites ported byte-exact; dispatch suite restored to the full 11-entry original; lazy-windowing geometry pins updated to the ratified overlay shape.
# Conflicts: # extensions/history/index.ts # extensions/history/selector-helpers.ts # tests/history-command-registration.test.ts # tests/history-openflow-integration.test.ts # tests/history-session-writer.test.ts
# Conflicts: # extensions/history/index.ts # extensions/history/selector-helpers.ts # extensions/history/store.ts
…_HISTORY_ENABLE, hidden.json cap Rework of the delete affordance per the slice-5 review decisions: - Two-step modal: ctrl+shift+backspace arms the selected row with the footer copy; while armed ONLY y/Y (execute), n/N and Esc (cancel) are honored — every other key is swallowed and stays armed, so nothing is ever typed into the search input. Esc while armed cancels the confirmation without closing the overlay. Wheel still disarms then scrolls. - Session-derived rows are read-only: the delete key on a session row is a silent no-op — session transcripts are immutable input owned by Pi core, so no hide, no tombstone, no store write comes from them. - Confirm copy: "Delete this prompt from history (y/n)? Prompt stays in session log" (single variant; the per-provenance distinction lives in docs/prompt-history.md). - Env rename: GENTLE_PI_HISTORY_CAPTURE → GENTLE_PI_HISTORY_ENABLE (captureEnabled, open-flow warning, docs, tests). - hidden.json retention: capped at HIDE_FILE_MAX_ENTRIES = 1000 in insertion order (newest last) — re-hiding refreshes recency, past the cap the oldest entries drop; .sort() removed; the fail-closed reader is unchanged, so the tombstone still applies to seeded copies and no prompt is permanently fixed. - docs/prompt-history.md: §Delete rewritten (modal, read-only session rows, verbatim failure toasts, cap); env renamed across all sections; stale "deletion UI is not shipped yet" sentences fixed.
# Conflicts: # docs/prompt-history.md # extensions/history/index.ts # extensions/history/selector-helpers.ts # tests/history-delete-backfill.test.ts
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (41)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Integration: history slices 1–6 + selection correctness
Integration branch merging, in stack order, the six history slices with all
review fixes applied, plus
feat/selection-correctness:hidden.json, DrainResult drains)feat/selection-correctness— native text selection engineValidation