feat: Allow user to press tab and add aditional context when denying.#1
Conversation
|
Hey @wviana — thanks a lot for this. You're literally the first PR on this repo. The Claude-Code-style "press tab and add a reason" UX is well-thought-out. A few notes: Heads-up on conflicts. I just landed CI is currently failing (https://github.com/esengine/reasonix/actions/runs/25087905451). Three Biome diagnostics, all auto-fixable:
Could you rebase onto |
Three significant fixes / additions on top of 0.13.1:
1. **Row-granular scrolling.** `sliceVisibleEvents` is replaced by a
pre-flattened row pipeline in `log-rows.tsx`. Each migrated event
role decomposes into one or more `LogRow`s (each ≈ one terminal
line); unmigrated roles fall back to a multi-row `LogBlock` that
wraps the legacy `<EventRow>` component. Migrated this commit:
· info / warning / error / step-progress
· finished assistant turns (header + body lines + stats line)
The slicer picks items by ROW range, the renderer wraps each item
in `<Box height={1}>`, and `overflow="hidden"` + `justifyContent`
handle clipping. Wheel = 3 rows / tick, PgUp/Dn = ~viewport.
Append-anchor sums row counts (not event counts) so the user's
logical scroll position is preserved exactly when content arrives.
The earlier negative-marginTop attempt is gone — Ink's overflow=
hidden does NOT reliably clip negative-margin children (verified
empirically), which let log content bleed up into the chrome and
hide the cost / cache pills. The pre-flatten approach avoids the
layout fight entirely.
2. **Vertical scrollbar + bottom-hint.** A 1-cell column on the
right of the log viewport renders a brand-colored thumb whose
position and size track the visible row range. Hidden when
`totalRows <= viewportRows`. The scrollbar component clamps its
own offset internally so a momentarily-stale `logScrollOffset`
can't make the thumb shrink toward zero in "scrolling into
emptiness" territory. Paired with a `BottomHint` row that surfaces
"↓ N rows below — End to jump" whenever the user is scrolled up,
so newly-arrived content at the bottom is never silently missed.
3. **No more bogus "aborted by user (Esc)".** When the event loop
was blocked >250ms (heavy Ink render on a long log), a
multi-byte sequence like `\x1b` + `[A` (Up arrow) split across
chunks would dispatch as `escape` THEN `upArrow` because the
`setTimeout`-based ESC ambiguity timer fires in the timers phase
ahead of the poll phase where stdin data events queue. The
spurious `escape` triggered `loop.abort()`, killing the running
turn — the same thing the PR #1 author reported as "sometimes
it's saying the user had aborted by pressing esc when I didn't".
Fix: defer the timer's dispatch via `setImmediate`, which runs in
the CHECK phase after POLL — giving any queued data events a
chance to consume the rest of the sequence first. The chunk
handler's `cancelEscTimer` already cancels both the timeout and
the immediate, so a real follow-up byte still wins.
Other polish:
· `maxScrollRows = totalRows - viewportRows` (was `totalRows - 1`)
so Home / max scroll lands on a fully-populated viewport showing
the start of the log instead of one row visible and the rest
empty.
· `useEffect` clamp on `logScrollOffset` snaps it back to
`scrollMaxRowsRef.current` when role-rendering height drifts
mid-stream — keeps the thumb in sync with reality.
Tests: 1625/1625 pass. Lint clean. Typecheck clean.
wviana
force-pushed
the
feat/allow-context-on-deny-tool-call
branch
from
007e0df to
7d9447b
Compare
Multi-paragraph docstrings + restated-what comments shipped via PR #1 trimmed back per CLAUDE.md.
…nt-policy gate feat(core): replay() reads events.jsonl sidecar and runs the same pure reducers as in-process apply() — first deterministic proof of the v0.14 projection layer. feat(cli): reasonix events <name> for inspecting any session's event stream from the command line. feat(ui): deny-with-context shipped via PR #1 (@wviana) — Tab on a tool-confirm modal opens inline reason entry, forwarded to the model so it knows *why* the user refused. chore: tests/comment-policy.test.ts enforces CLAUDE.md rules under npm run verify; companion sweep removed 6.3k LoC of dead-weight comments across 148 files.
|
@esengine could you enable discussions on this repo so we can chat about things that would not yet fit as issue? |
|
@wviana Discussions is already enabled — categories are live (General / Ideas / Q&A / Show and tell / etc.). The Ctrl+C-as-interrupt + double-Esc-to-abort idea fits Ideas perfectly; would love to dig into it there. FWIW I lean the same way — Ctrl+C should cancel the in-flight turn, not nuke the app. |
2 participants
Hi there, I got really interested on this project.
This MR is just one pain that I felt when using it. On work I'm used to use claude code. On it there is this feature when you're answering a a prompt with no, that is equivalent to deny here on reasonix.
So I was playing with reasonix and sometimes I've faced it trying to do something where I would like to say no but add some additional context on what it should do instead. But the deny I had was saying to the model to keep going without running that command/tool. This is how I come with this change. This was all coded using reasonix itself.
So this is pretty much the same as on claude, you press tab, it will add a
,and allow you to write additional context.I've only manually tested it, I think you don't yet have tests for those ink components yet.
I'm not expecting to get this merged right away, but just a way to start to get in touch.
There are some other things I would like to improve on it, but I'll make the changes and open new MRs for those. Mostly some UI improvements and sometimes it's saying the user had aborted by pressing esc when I didn't.
But again, just a first contact to get to know if PRs are welcome.