Skip to content

Replace the side-panel Inspector tree with a tabbed Inspector that needs no server support - #565

Open
ericwinger wants to merge 3 commits into
mainfrom
eric/issue508-tabbed-inspector
Open

Replace the side-panel Inspector tree with a tabbed Inspector that needs no server support#565
ericwinger wants to merge 3 commits into
mainfrom
eric/issue508-tabbed-inspector

Conversation

@ericwinger

Copy link
Copy Markdown
Member

Fixes #508

The Enhanced Inspector needs its server-side payload installed and a stone at 3.7.5 or later,
so on a 3.6.x stone Inspect It fell back to a read-only tree in the sidebar no matter what was
installed. That tree is removed outright, along with its Remove and Clear All commands, and
replaced by a basic Inspector that opens as an editor tab in the Enhanced Inspector's own idiom —
tabs over a miller-column strip — built entirely on GCI primitives and kernel sends, so it needs
no server support and works back to 3.6.2.

The Enhanced Inspector is untouched: routeInspect keeps its existing rule, and the only file it
shares in this change is the Meta-tab scroll fix, which it had the same bug in.

What it does

An object gets only the tabs its structure warrants: Slots, Items or Entries,
Bytes, Print, Meta and Evaluate.

  • Double-click a row to open that object to the right; Enter dives in place. Each column carries
    its own Back/Forward.
  • A row's context menu copies its printString or OOP, browses its class, and edits its value
    through instVarAt:put:, at:put: or a dictionary key, with a revert to the value from before
    the first edit.
  • Slots list alphabetically, sorted by clicking the Name column. Each row keeps the
    instVarAt: index it was read at, so sorting cannot misdirect a write, and declaration order —
    what allInstVarNames answers — is that index ascending.
  • Meta shows the class name and an info bar of superclass, package and OOP, then sub-tabs for
    instance methods, class methods, definition and comment.
  • The Evaluate pane answers to the editor's own Ctrl+K D / E / I for Display, Execute and Inspect
    It, and lists the names in scope beside the expression: the receiver, then every instance
    variable grouped under the class that declares it. Debug It is absent — it works by compiling
    with the single-step flag set, and the pane's evaluateInContext:symbolList: runs to
    completion, leaving no halted process to attach to.
  • Every paged tab offers Load all beside Load more, bounded at 50 pages a click so a
    million-element collection cannot hold the session; the tab keeps showing a remainder and
    another click carries on.
  • Panels are tracked per session and disposed together when that session logs out — the
    replacement for the tree's per-session root removal.

The column strip both inspectors were growing separately is now one module,
client/src/webview/millerColumns.js, which owns order, insert-right drilling, close, focus and
width; each inspector supplies what is drawn inside a column.

Acceptance criteria

  • The side-panel inspector presents an object as tabs, in the Enhanced Inspector's visual
    idiom.
  • Works against a stone with no server support installed, including 3.6.x, using only GCI
    primitives.
  • Every capability of the classic tree still works or has a stated replacement — multiple
    roots become multiple editor tabs, per-session logout disposal replaces per-session root
    removal, and the arrival badge/status-bar flash is replaced by the tab opening.
  • The Enhanced Inspector keeps working exactly as it does today.

Follow-ups

The wish-list items in #508 that are not in this change are collected in #564 — structural
editing of collections and dictionaries, search/filter within a tab, multi-select, dynamic
instance variables, inspecting a dictionary key separately from its value, and a status line.

Testing

  • npx vitest run src/basicInspector — 185 tests across 3 files, all passing.
  • tsc -p client --noEmit and eslint client/src/basicInspector both clean.
  • Coverage was measured by recording every function entry across a run rather than assumed; the
    two paths no test reached — dive and the fetchObjectMeta wrapper — now have tests.
  • Exercised by hand against a live stone.

🤖 Generated with Claude Code

ericwinger and others added 2 commits September 4, 2026 11:25
The Enhanced Inspector needs its server-side payload installed and a stone
at 3.7.5 or later, so on a 3.6.x stone Inspect It fell back to a read-only
tree in the sidebar no matter what was installed. That tree is removed
outright, with its Remove and Clear All commands, and replaced by a basic
Inspector that opens as an editor tab in the Enhanced Inspector's own idiom
— tabs over a miller-column strip — built entirely on GCI primitives and
kernel sends, so it needs no server support and works back to 3.6.2.

An object gets only the tabs its structure warrants: Slots, Items or
Entries, Bytes, Print, Meta and Evaluate. Double-click a row to open that
object to the right, Enter to dive in place; each column carries its own
Back/Forward. A row's context menu copies its printString or OOP, browses
its class, and edits its value through instVarAt:put:, at:put: or a
dictionary key, with a revert to the value from before the first edit.

The column strip both inspectors were growing separately is now one module,
webview/millerColumns.js, which owns order, insert-right drilling, close,
focus and width; each inspector supplies what is drawn inside a column.

From testing the tabs:

- The Evaluate pane answers to the editor's own Ctrl+K D / E / I for
  Display, Execute and Inspect It. The contributed bindings cannot serve —
  all three are when: editorTextFocus, and the commands behind them read
  the active text editor — so the pane recognises the chord itself. Debug
  It is absent: it works by compiling with the single-step flag set, and
  the pane's evaluateInContext:symbolList: runs to completion, leaving no
  halted process for a debugger to attach to.

- Meta lists each selector as a row — a rule under it, a caret marking the
  one whose source is open — rather than a paragraph of words, and opening
  one no longer throws the reader back to the top of a long list. The
  Enhanced Inspector's Meta tab had the same jump, and is fixed with it.

- The Bytes dump was lying about its own offset column: a decimal 1-based
  index, zero-padded to eight, reads as a hex offset next to a field of
  hex. It now carries a column header, a plain decimal index that ties a
  byte back to the element it belongs to, a Hex/Dec switch, and the spaces
  that align the columns actually survive rendering.

- Every paged tab offers Load all beside Load more, bounded at 50 pages a
  click so a million-element collection cannot hold the session; the tab
  keeps showing a remainder and another click carries on. Bytes says how
  much of the object it is showing even when that is all of it, which is
  what made its paging look absent on anything under one page.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Editor tabs follow Jadeite's own inspector caption — the class, plus
(N characters) for a String and (N bytes) for a ByteArray. They were named
from the label, which after an Inspect It in a workspace is the expression
you selected, so every tab read as a wall of source. A printString is no
better on a tab strip: as long as the object cares to be, and near-identical
between two instances of a class. The expression still names the column,
where "what produced this" is the useful thing.

Slots list alphabetically, sorted by clicking the Name column rather than a
row of buttons above the table. The stone answers allInstVarNames, which is
superclass-first declaration order — unsearchable by eye on a real class —
and that order is still one click away for when the class, not the value, is
what you are reading. Each row keeps the instVarAt: index it was read at, so
sorting cannot misdirect a write, and declaration order is that index
ascending.

Meta is laid out as the Enhanced Inspector lays its own out: the class name,
an info bar of superclass, package and OOP, then sub-tabs for instance
methods, class methods, definition and comment. Stacking definition and
comment above the selectors cost the selector list its screen — a class with
a real comment pushed every method below the fold.

The Evaluate pane fills the space beside the expression with the names in
scope: the receiver, then every instance variable grouped under the class
that declares it, since allInstVarNames spans the whole chain and an
inherited variable is as writable here as any other. Each carries the class
it currently holds, and self carries how much of the object there is, so an
Array or String — which has no named instance variables at all — says what
`self at:` has to work with. Click a name to type it at the caret, or copy
it. The clear button is the ✕ inside the box that the debugger's eval bar
and the list filters already use.

Fixed: editing a value took you to an inspector on it. Committing clears
`editing`, which is the guard the strip's Enter handler checks, so the
keystroke that committed the write was then read as "dive into the selected
row" — editing a Character of a String replaced the column with the
Character. Enter in a field now stops there and is never a dive.

Coverage was measured by recording every function entry across a run, rather
than assumed. Two things no test reached: `dive`, whose two gestures — Enter
on a selected row, and the context menu — were never exercised because the
history tests feed the panel's reply straight in; and the `fetchObjectMeta`
wrapper, whose parser was covered but whose doit and refusal path were not.
Both now have tests, and a test claiming to dive that actually clicked
Inspect is renamed to what it does.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@ericwinger
ericwinger requested a review from npapagna September 4, 2026 23:48

@github-advanced-security github-advanced-security AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CodeQL found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

…a prototype

The merge conflicted only in CHANGELOG.md, where both sides had appended to
Added and to Fixed; both sets of entries are kept.

CodeQL raised 30 prototype-pollution alerts against basicInspectorView.js, all
of them the same finding reached by 30 paths. The column strip indexes its
columns in a plain object, and `get` is handed ids that arrive from outside the
model — a host message's `columnId`, and a DOM dataset in `columnOf`. Ask a
plain object for `__proto__` and it answers Object.prototype, so the renderer's
next line — `col.oop = ...`, `col.title = ...`, twenty-eight more — would write
a column's worth of fields onto the prototype of every object in the frame.
CodeQL flagged each of those assignments, which is why one map produced thirty
alerts.

The map is now `Object.create(null)`, which answers undefined for those keys;
every caller already treats undefined as "no such column". The three per-column
maps keyed by strings off the same messages — `tabData` and `loadedRows` by tab
name, `methodSource` by selector — get the same treatment in both the place
that seeds them and the place that resets them.

No behavior changes: these are lookup tables, nothing iterated them expecting
inherited keys, and the only hasOwnProperty in the module is the guarded
`Object.prototype.hasOwnProperty.call` form against a different object.

Three tests pin it at the model, where the fix lives, so it holds for the
Enhanced Inspector's strip too: a prototype key answers undefined, a child
message naming `__proto__` as its source appends at the far right and leaves
bystander objects clean, and a real id still finds its column.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@ericwinger

Copy link
Copy Markdown
Member Author

CodeQL's 30 alerts were one finding reached by 30 paths, and it was right. Fixed in bb8425c.

The column strip indexed its columns in a plain {}, and Columns.get(id) is handed ids that
come from outside the model — a host message's columnId, and colEl.dataset.colId in
columnOf. Ask a plain object for __proto__ and it hands back Object.prototype, so the very
next line in populateColumncol.oop = …, col.title = …, and twenty-eight more — would
have written a column's worth of fields onto the prototype of every object in the frame. Each of
those assignments is what CodeQL flagged, which is why one map produced thirty alerts.

columnsById is now Object.create(null), which answers undefined for those keys; every
caller already treats undefined as "no such column" and returns. The three per-column maps
keyed off the same messages get the same treatment — tabData and loadedRows by tab name,
methodSource by selector — in both the place that seeds them and the place that resets them.

No behavior change: these are lookup tables, nothing iterates them expecting inherited keys, and
the module's only hasOwnProperty is the guarded Object.prototype.hasOwnProperty.call form
against a different object.

Three tests pin it in millerColumns.test.ts, at the model rather than in this inspector, so it
holds for the Enhanced Inspector's strip too: a prototype key answers undefined, a child
message naming __proto__ as its source appends at the far right and leaves bystander objects
clean, and a real id still finds its column.

Worth noting for anyone reading the check history: this push is also the first time the
Health Check workflow ran on this PR. The branch was conflicting with main, so GitHub could
not build the merge ref the pull_request trigger needs, and CodeQL — which analyzes
refs/pull/565/head instead — was the only thing that ran. The merge with main in bb8425c
conflicted only in CHANGELOG.md, where both sides had appended to Added and to Fixed; both sets
of entries are kept.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Replace the side-panel Inspector with a tabbed inspector that needs no server support

2 participants