feat(review): review workspace - #188
Draft
matej21 wants to merge 64 commits into
Draft
Conversation
…erlay Pure key dispatch over (screen, focus region, input focus, search state) covering the spec table: navigator modes, filter, roles menu, queue and symbol stepping, details, split/whitespace, find, copy, help; keys are swallowed while a text field has focus. Esc ladder: help -> popovers -> filter -> find -> legacy transients -> overview -> close. Footer shows only hints that work on the current screen. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Tb7BZX8NnJ6nU8qHqzRvwL
Review fixes: the dismiss backdrop occludes (a click outside no longer also selects the row underneath), Unavailable has no details link, pill and popover use the same noun, detail column ellipsizes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Tb7BZX8NnJ6nU8qHqzRvwL
build_review_model now derives file and symbol entries, deterministic tiers (contract, behaviour, volume, git facts, rest) with spelled-out reasons, the deduplicated attention list, directory aggregation with no-test-changes detection, volume by role, facts (public API, tests, moves, commits, also), analysis status precedence, omission sentences and coverage. Shared test fixtures cover renames, deletions, binaries, config, public removals and signature+body+call changes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Tb7BZX8NnJ6nU8qHqzRvwL
Review fixes: drop Ctrl+1/2 (taken by the global FocusSidebar binding, F6 remains), ignore alt/function/shift variants of review keys, resolve accelerators before punctuation, gate file-only keys to the file screen, return focus to the navigator when going back to the Overview, make the help card scroll, and pin the help table to what is actually bound. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Tb7BZX8NnJ6nU8qHqzRvwL
…pover File header states path, role (rule on hover), status, churn, reason chips, analysis state, outline link and queue position once. The symbol bar follows the changed symbol in view and expands to a normalized signature token diff and the calls changed in that function with their control context. Outline popover shows base and head outlines with changed symbols clickable. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Tb7BZX8NnJ6nU8qHqzRvwL
Change-at-a-glance headline, stacked bar and legend by role (clickable role filter); one-line facts for public API (lower-bound aware), tests, moves, commits and lockfiles/submodules/binaries; the ordered Start-here list with its coverage caveat; one-column reflow under 1000 px. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Tb7BZX8NnJ6nU8qHqzRvwL
…okups Review fixes for the ranking: Body comes from body_changed (the analyzer emits a ChangedLines hotspot for every changed function), no-test-changes marks only the top-most implementation directory (binaries never make an implementation directory), deleted/moved/config/lockfile/submodule/binary files keep their own attention row even when analyzed, LargeChurn only without symbol reasons, Behaviour tier only for modified symbols, and structure/call/hotspot lookups are indexed once instead of rescanned. Fixtures now mirror the producer's hotspot emission. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Tb7BZX8NnJ6nU8qHqzRvwL
Segmented Files/Attention control, filter box, one Roles button with a presets + all-roles + saved-filters menu, virtualized directory tree with visible-subset totals, joined chains, at most two reason markers per file and dimmed not-analyzed rows; the Attention list with tier separators, two-line reason rows, reason-chip filters and a group-by-file variant. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Tb7BZX8NnJ6nU8qHqzRvwL
Adds ledger_open to the review UI state and a toggle action; the Overview's Commits line gains show/hide ledger and lists commits oldest first with merge badges and relative dates. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Tb7BZX8NnJ6nU8qHqzRvwL
…in Attention Move the analysis pill into the header's control group so its popover anchors under it, stop propagation of keys the review handled so 1/2 never reach the app, and map the reveal-in-Attention scroll through the rendered rows (tier separators interleave the item list). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Tb7BZX8NnJ6nU8qHqzRvwL
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Tb7BZX8NnJ6nU8qHqzRvwL
Review fixes: Also link no longer widens to the whole Implementation role, headline never shows zero lines, tiny percentages read < 0.1 %, deletion-heavy changes are described in words, the path-order caveat is suppressed when it repeats itself, counts are digit-grouped, filter links replace instead of layering, and the facts column is skipped when empty. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Tb7BZX8NnJ6nU8qHqzRvwL
Review fixes: read the top visible row with the uniform-list idiom (item height from the measured contents, prefer a pending scroll) and map it through the side-by-side rows in split mode; fall back to the preceding symbol past the end; a selection holds only while the top row is within or just before it; step_symbol starts from the followed symbol so } and the counter agree; outline popover only in the file view, keyed by SymbolKey, anchored under the header; queue stepper stays visible when the target is filtered out. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Tb7BZX8NnJ6nU8qHqzRvwL
Review fixes: grouped-mode file headers carry the file key and open the file; the no-tests marker survives joined chains under a filter; one not-analyzed predicate for dimming, footer and roles menu; identity-based element ids; empty states for both modes; row ids pinned to the rendered rows. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Tb7BZX8NnJ6nU8qHqzRvwL
Seen on a real branch: the diff pane repeated the file path under the review header, lockfile/config rows carried both a reason chip and a role badge saying the same thing, and the coverage caveat said "structure reached 76 of 76 implementation files". Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Tb7BZX8NnJ6nU8qHqzRvwL
A callee that appears more than once inside a function used to degrade the whole group to removed + added, even when every occurrence read the same on both sides. Any edit above the group then reported "141 calls changed" for a +7/−8 change. Identical occurrences (arguments, control context, provenance) now cancel out first; a single changed occurrence left on each side pairs as Modified under a new UniqueChangedOccurrenceWithinEnclosingRange strategy, and anything still ambiguous keeps degrading to added/removed — nothing pairs by ordinal. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Tb7BZX8NnJ6nU8qHqzRvwL
The navigator re-scrolled to its cursor row on every render while it had focus, so the wheel snapped back the moment the cursor left the viewport and switching modes landed mid-list. The reveal is now a one-shot request (`nav_reveal`) set by whatever moved the cursor: ↑ ↓ (Nearest), ] [ (Nearest — the cursor now follows the queue), and a mode switch that parks the cursor on the open file (Center). Rows: the stripe is the keyboard cursor and the fill is what the content shows, instead of one merged "selected"; names truncate instead of wrapping onto a second line at narrow widths; a directory whose files all share one role carries the Tests/Docs badge once instead of every file; `body` no longer marks files (every edited function has one) and a new file's symbols add no markers of their own. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Tb7BZX8NnJ6nU8qHqzRvwL
} { and ] [ inside the open file went through the full open-file path:
a new file generation, "Loading exact source…", a re-highlight, and a
scroll reset for every step. When the target file is the one already
loaded and displayed, navigation now keeps the generation and maps
straight to the row.
The symbol bar also lost the selection right after such a step: the diff
centres the target, so the viewport *top* sits in the previous symbol and
the bar followed the view. The selection now holds while any of the
symbol is on screen (the bar reads the viewport bottom too, from the list
bounds and the uniform row height), so "changed symbol k of n" and the
next } agree.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Tb7BZX8NnJ6nU8qHqzRvwL
The details block under the symbol bar rendered nothing for most symbols (no signature change, no calls, not complex) and, when it did have calls, dumped their raw multi-line source in a wrapping flex — a wall of text for one useState(). Now it is a label column with one row per fact: Reasons (only when the bar could not fit them all), Lines (always — base/head span of the symbol's hunks), Signature, Calls, Complexity. Calls are one line each with whitespace collapsed and long text cut, removed and modified first, capped at eight with "… n more". A modified call whose arguments did not change reads once, with the branch move in its context (`in loop → loop · closure`); CallRow carries the base-side context for that. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Tb7BZX8NnJ6nU8qHqzRvwL
The panel clipped at its max height with no scroll container, so long outlines were unreachable. Both columns now scroll together inside the panel; the panel grew to 520 px. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Tb7BZX8NnJ6nU8qHqzRvwL
The overview stretched to the window: legend numbers drifted to the far edge, the facts column with them, and Start-here reasons started at a different x on every row. The page now stops at 1080 px, the legend's role column is fixed so its numbers sit beside it, and Start-here rows put name + file in one fixed block so the reasons and the churn line up. The file next to a symbol reads `…/dir/file.rs` (full path in the tooltip); a file row shows its directory the same way. Remaining single-line labels truncate instead of wrapping. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Tb7BZX8NnJ6nU8qHqzRvwL
"Start here" is what the Overview is for, so it takes the left column and gets the room: two lines a row — name and churn, then path and reasons — hairline separated instead of five columns squeezed onto one line. The composition and the facts move into a 380 px sidebar on the right, where the legend label flexes and its numbers sit right next to it. The legend stops at 420 px so the stacked layout below 1 000 px does not pull the numbers away from their roles again. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Tb7BZX8NnJ6nU8qHqzRvwL
The navigator is about to print the same call changes the details bar does, so the wording moves to `labels::calls` where both can read it. `text::call_lines` and friends stay as re-exports, so the bar is untouched. `signature_pair` is new: one line for a column too narrow for the two-line token diff. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Tb7BZX8NnJ6nU8qHqzRvwL
A callee used four times the same way produced four identical lines, which is four lines of nothing. Occurrences that read identically — same callee, arguments and branch — now share one line with `×4`, and the limit counts lines rather than occurrences, so a symbol's few real changes are not pushed behind its repetition. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Tb7BZX8NnJ6nU8qHqzRvwL
One switch — `outline`, or `e` — puts every visible file's changed symbols under it, and under each symbol the lines that say what changed: the signature pair, then the calls with the branch each sits in. No expanding file by file: the whole change reads by scrolling, which is what the tree was missing between "which files" and "open one and read the diff". Symbol rows open their symbol and carry only the markers the lines below do not already state — `body` and `calls` are dropped, since the lines under the row *are* the calls. Detail lines open the same symbol but the cursor steps over them, so `↑` `↓` still walk file to symbol to symbol. The footer counts them, because the switch is what decides whether the list is 89 rows or 4 000. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Tb7BZX8NnJ6nU8qHqzRvwL
Five levels of 12 px indent read as one wall of text, so the outline lost the shape it was supposed to show. Every row now draws a hairline per level it hangs under, detail lines sit one level below their symbol rather than beside it — the guide then runs down the symbol's own glyph column and the block reads as the symbol's — and a file with an outline under it is filled, so it reads as the header of that block. The header is a fill and not a rule on purpose: a border would make those rows a pixel taller than the rest, and the virtualized list measures one height for all of them. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Tb7BZX8NnJ6nU8qHqzRvwL
The outline listed every changed symbol of a file at one level, so a class and its methods were siblings and the shape of the class was lost. A symbol whose enclosing symbol also changed now sits one level under it — one level per enclosing change — and the class reads as its own outline. A member of a symbol that was *removed* whole is dropped: an interface with eighteen fields produced nineteen `removed` rows saying the same thing. New members stay, because a new type's members are what the type actually is. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Tb7BZX8NnJ6nU8qHqzRvwL
Roles are decided by path, so a Rust repo that keeps its tests in `#[cfg(test)] mod tests` had no tests at all: 97.5 % implementation, "no tests changed next to any of the 9 implementation directories", and every test case ranked as an implementation change. All three were wrong about this branch. Structure already sees those scopes — the module is a symbol and its cases carry it in their qualified name — so a symbol whose scope chain holds a test name is a test change. Its lines count once, on the outermost such scope, or a changed `mod tests` would count its cases twice. From that: the Tests row of the legend gets the lines back (a role may now have lines without files of its own, `in 47 files`), an implementation directory with inline tests is no longer untested, the `tests` filter hides those symbols like it hides test files, and the outermost test scope carries the `Tests` badge in the outline. The names are the heuristic — `mod tests`, `spec`, the same vocabulary the path rules use for directories. Reading the `#[cfg(test)]` attribute itself would need the extractor to keep attributes; this is the same answer without a wire change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Tb7BZX8NnJ6nU8qHqzRvwL
matej21
added a commit
that referenced
this pull request
Aug 18, 2026
`okena-files` owned the syntect setup — the shared `SyntaxSet`, the theme pair, the per-line span builder — and `okena-views-git` reached into it for the diff viewer. That works while only viewers need colour, but the markdown renderer needs it too, and `okena-files` already depends on `okena-markdown`, so the dependency cannot run that way. `okena-highlight` now sits below both. `syntax.rs` and `markdown_highlight.rs` move across unchanged; `build_styled_text_with_backgrounds` moves out of `code_view` for the same reason. `okena-files` re-exports all three, so every `okena_files::syntax::…` and `code_view::build_styled_text_with_backgrounds` import keeps working. Loading the `SyntaxSet` twice would have cost megabytes for nothing, which is what a second copy in `okena-markdown` would have meant. The name says highlighting rather than syntax because #188 is adding an `okena-syntax` for something else — tree-sitter facts about code structure, no colours involved. `highlight_code_block` is the entry point a fenced block needs: it resolves a language token rather than a file path, and loads the `SyntaxSet` itself since the markdown renderer holds no file. It returns nothing for a fence with no language or one syntect cannot place — plain-text highlighting would repaint such a block in the syntax theme's foreground, and a block with nothing to colour should keep the document's own text colour. Tabs are left alone there: the caller maps character offsets onto the spans, so expanding a tab to four spaces would shift every selection on the line. The file path keeps expanding them, as it always has. `syntax_for_lang` in `markdown_highlight` was the same mapping as the new resolver and is now that resolver. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AWSp87PWoqnyB6prN3U3qD
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.
Summary
okena-syntax(bounded Rust/TypeScript fact extraction),okena-review(classification, structural comparison, call diff), exact source loading through the daemon, and the whole UI inokena-views-git/src/diff_viewer/review_ui.Design of record:
docs/review-workspace-ui-spec.md(presentation) overdocs/review-workspace-product-plan.md(background). Both are in this branch.What it does
Overview — the ordered list is the page, the composition its sidebar: implementation volume vs supporting volume by role, and ten rows that each name the reasons that placed them. Facts state Public API, tests, moves, commits and lockfiles; every one of them links somewhere.
Navigator — a Files tree and an Attention list over the same ranking.
outline(keye) inlines every file's changed symbols and, under each, what changed inside it: the signature pair, then the calls with the branch each sits in. No expanding file by file — the whole change reads by scrolling.File view — the diff, with the changed symbol's signature, call changes and complexity one keystroke away (
d),}{to step symbols,][to walk the attention queue from any file.Honesty rules the spec holds the implementation to: structure-derived counts are lower bounds (
≥ N) while coverage is partial, a capped run is “limited” and never “complete”, no zero-valued cells, no debug enum names on screen, and every row opens something.Notable decisions
×4. Without it a +6/−8 file reported “141 calls changed”.#[cfg(test)] mod testslooked 97.5 % implementation with “no tests changed next to any of the 9 implementation directories”. A symbol whose scope chain holds a test name is a test change; its lines count once, on the outermost such scope. The names are the heuristic — reading the attribute itself would need the extractor to keep attributes.Validation
cargo test --workspace— 2230 passed, 0 failed.cargo clippy --all-targets -- -D warnings— clean on the crates this touches.Not in this iteration
Notes / reviewed state, evidence links, AI summaries, PR targets, working-tree and staged targets, per-commit file lists, “open commit diff” per ledger row. The shell reserves space for them.
Alt+↓Alt+↑(hunk stepping) is reserved and unbound.Draft: the branch is large and worth reading in commit order — the pipeline crates land first, the UI last.
🤖 Generated with Claude Code
https://claude.ai/code/session_01Tb7BZX8NnJ6nU8qHqzRvwL