feat(btw): add branch-local history and continuity - #880
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6649ed0401
ℹ️ 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".
| import { formatBtwQuestion, sanitizeBtwDisplayText } from "./display-text.ts"; | ||
| import { type BtwHistoryViewEntry, BtwHistoryViewModel } from "./history-view-model.ts"; | ||
|
|
||
| const FOOTER_HINT = "left/right: question up/down: scroll esc: close"; |
There was a problem hiding this comment.
When any navigation or cancel binding is remapped in keybindings.json, KeybindingsManager replaces its default keys, but this footer still advertises Left/Right, Up/Down, and Escape. The overlay therefore tells users to press keys that no longer work; render the hints from the provided keybinding manager rather than a hardcoded constant.
AGENTS.md reference: AGENTS.md:L99-L99
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in c54a3e1. The footer now derives left/right, up/down, and cancel labels from the injected KeybindingsManager via the shared formatKeyText helper. Added a RED->GREEN regression for remapped ctrl+h/l/k/j/x labels. Verification: focused /btw suite 39/39, npm run check, full build, targeted regression PASS, and real ConPTY 15/15 including the effective-key footer.
|
CI and the changelog gate were created but are waiting for maintainer approval before any jobs can start. Please approve these current-head fork workflow runs when convenient:
The published head is |
|
Codex Review: Didn't find any major issues. Bravo. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5002a07295
ℹ️ 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".
|
|
||
| function formatFooterHint(keybindings: KeybindingsManager): string { | ||
| const keys = (binding: Keybinding): string => formatKeyText(keybindings.getKeys(binding).join("/")); | ||
| return sanitizeBtwDisplayText( |
There was a problem hiding this comment.
When a configured key label contains a JSON-escaped \n or \r (for example, "ctrl+x\nFORGED"), sanitizeBtwDisplayText() preserves the newline and this footer returns it inside a single Component.render() row. The TUI compositor treats each array element as one physical row, so the embedded newline is written directly to the terminal, allowing a malformed or malicious keybindings.json to inject an extra row and desynchronize differential rendering. Use a one-line sanitizer here that removes or collapses CR/LF as well as terminal escape sequences.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in f320b69. The configured footer now uses the existing one-line display formatter after terminal-control sanitization, collapsing CR/LF before the component row reaches the TUI compositor. The JSON-boundary regression now combines OSC 52, BEL, and a newline-bearing FORGED label and asserts the footer element contains no CR/LF. Verification: focused suite 40/40, npm run check, full build, and real malicious-config ConPTY 15/15 with auth unchanged.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f320b69416
ℹ️ 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".
| role: "user", | ||
| content: `Earlier side question: ${entry.question}\nYour earlier answer: ${entry.answer}`, |
There was a problem hiding this comment.
When an earlier side-query answer contains imperative text or a prompt-injection payload, the next /btw request sends that model-generated answer with role: "user", promoting it to user-authority instead of preserving it as an assistant reply. This can incorrectly steer subsequent side answers, especially after switching models or providers; represent each stored question and answer with their original user/assistant roles rather than combining both into a user message.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in a6e13e2. Each stored side question is now replayed as user, followed by its model-generated answer as assistant; selected-model metadata supplies the provider-neutral assistant message envelope. A RED-to-GREEN extension regression inspects the real faux-provider call log with an imperative prior answer, and the localhost RPC wire driver confirms the prior answer remains assistant. Verification: focused 41/41, npm run check, full build, RPC 20/20, tools empty, current question last, auth unchanged.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a6e13e28a7
ℹ️ 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".
| { role: "user", content: `Earlier side question: ${entry.question}`, timestamp: entry.timestamp }, | ||
| { | ||
| role: "assistant", | ||
| content: [{ type: "text", text: `Your earlier answer: ${entry.answer}` }], |
There was a problem hiding this comment.
When context budgeting is triggered and removing an earlier side-question message is enough to fit, pruneOldMessagesToBudget() removes that user message independently and stops, leaving its assistant reply in the provider context without the originating question. Fresh evidence at the current head is this new two-message representation combined with the existing per-message pruning in side-query.ts; the large-context path therefore no longer preserves the turn provenance this fix establishes. Keep each stored question/answer pair atomic during pruning, or remove orphaned side-history messages afterward.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in d73a092. After generic model-aware pruning and tool-pair repair, the /btw bounder now drops any incomplete prefix before the first surviving user turn, so a prior assistant answer cannot remain after its question is removed. The RED test calculates the exact token boundary that previously produced [assistant prior answer, user current question]; GREEN yields only the current user question. Verification: focused 42/42, npm run check, full build, standalone production bounder PASS, RPC 20/20.
|
Codex Review: Didn't find any major issues. What shall we delve into next? Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
1 participant
Summary
/btwside questions now keep branch-local history without adding those exchanges to the main model conversation. Completed questions and answers survive session reloads, a bare/btwopens a keyboard-driven history viewer, and new side questions can refer to the newest ten earlier side exchanges.The change is extension-local and preserves the current model-aware side-query context reducer: main history, prior side answers, and the current question are budgeted together before provider dispatch.
This replaces #746 with a clean branch based directly on current
main; the previous branch had absorbed an old main lineage and was no longer a reviewable feature diff.Motivation
Side questions are useful precisely because they do not derail the main task. Before this change, they were ephemeral: users could not review an earlier answer, and a follow-up side question had no memory of the previous side exchange.
Persisting custom entries at the builtin extension boundary provides continuity and review while keeping the main LLM transcript untouched. Reading history from
sessionManager.getBranch()prevents exchanges from sibling branches from leaking into either the viewer or follow-up context.Behavior
/btw <question>stores the completed question and answer as abtw-historycustom entry.userrole and prior model answers retainassistantrole in follow-up provider context./btwopens the history overlay in TUI mode and emits the same list throughnotifyin non-TUI modes without calling a provider.tools: [], isolated provider session IDs, concurrent main-turn behavior, and current context-window budgeting remain unchanged.Test evidence
Passed locally on Windows on current PR head
d73a092b7:npm run check: passed (Biome, TypeScript, lock audits, browser smoke).npm run build: passed across all workspace phases./btwscenario with a localhost model: 20/20, including prior question=userand answer=assistantwire roles, sanitized live output, persisted-history replay, provider-error notification, and unchanged provider continuity.127.0.0.1.QA receipt:
local-ignore/qa-evidence/20260813-btw-history/.The package-wide suite remains red on this Windows checkout for unrelated pre-existing platform assumptions (Unix sockets and chmod, POSIX path separators, shell/native publish fixtures, app-server timing). The changed
/btwsuite passes in isolation and no full-suite failure names a changed/btwpath.MacBook verification
The implementation uses the shared
pi-tuikey/width helpers and standard Node session APIs, with no Windows-only product path. macOS execution is not claimed as completed in this PR. To verify from this branch on a MacBook:Then run Senpi from source and verify in Terminal.app plus Ghostty or another Kitty-protocol terminal:
/btw <question>./btwand use Left/Right, Up/Down, and Escape.Risks and residuals
/btwwidget still uses its documented raw-Escape behavior and is not changed here.packages/coding-agent/CHANGELOG.md## [Unreleased]; the local changelog gate passes.