Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 46 additions & 37 deletions CONTEXT.md
Original file line number Diff line number Diff line change
Expand Up @@ -298,45 +298,54 @@ buckets its integer `(file, line)` notes into single-line spans.

In the viewer a span is a label, and it shows only where its code does
(see [[fold-level]]). At `files` and `hunks` nothing mentions a span. At
`code` a span lives in the **centre gutter** — the new half's sticky
columns after its line numbers, `lineno · text · bars`, a fixed width
`code` a span lives in the **span gutter** at the diff's right edge —
the new half's last two columns, `lineno · code · bars · text`, sticky
to the half's right edge as the line numbers are to its left, so the
strip stays put while the code scrolls under it. It has a fixed width
per file (26ch of text, one 6px bar column per level of nesting; nothing
for a file with no span) split between the halves; the bars sit against
the code they mark. Every span takes one form, whatever its length: a
mark in the bars column — a bar over exactly its rows, or a dot on a
span of one line, one column further right per level of nesting — and
a text block in the text column, at a smaller size, wrapping at the
gutter's width, that starts on the span's first row and hangs down for
as long as it needs — past the end of its bar if it must; nothing is
for a file with no span), split between the halves so the two code
viewports stay equal and sit beside each other; the bars sit against the
code they mark, the text outside them. Every span takes one form,
whatever its length: a mark in the bars column — a bar over exactly its
rows, or a dot on a span of one line; the outermost span's column is
against the text and each level of nesting is one column nearer the code
— and a text block in the text column, at a smaller size, wrapping at
the gutter's width, that starts on the span's first row and hangs down
for as long as it needs — past the end of its bar if it must; nothing is
clipped, and nothing of a span is in the code column. The block leads
with the span's smell pills, so they sit beside the bar's first row,
then the intent. The block is a zero-height grid item, so it sizes no
row; the one placement rule is that blocks do not overlap. A downward
pass over the half's rows (`render._layoutSpanTexts`) enforces it:
where a block would start inside the one above, the nearest code row
with a pill row, so it sits beside the bar's first row — the span's
smell pills, each promotable to a [[reviewer-comment]] by a click, and a
`+ comment` affordance that promotes the span's intent the same way, by
the one-click path (`Comments.promote`), as a comment on the span's
first line — then the intent. The block is a zero-height grid item, so
it sizes no row; the one placement rule is that blocks do not overlap. A
downward pass over the half's rows (`render._layoutSpanTexts`) enforces
it: where a block would start inside the one above, the nearest code row
above it is stretched by the overlap (a `min-height`, mirrored onto the
old half's paired row so the halves stay aligned), and text running
past the hunk's last row stretches its last code row; every other row
keeps its natural height. Two spans starting on one row chain their
blocks, outermost first. The same pass gives every non-code row inside
a bar — a comment thread's row, a fold's label row — a bars cell with
the bar's segment at its depth, so a bar runs unbroken through them.
The pass runs when the half's rows change (an annotation inserted, a
fold hiding rows — a `MutationObserver`) or its size does (a
`ResizeObserver`, on the next frame); it measures once, writes once,
and discards the mutation records its own writes queue. When a fold
hides a span's first row its text hides too and the fold's label tree
lists it ([[fold-region]]). A span the reviewer has turned into a
[[reviewer-comment]] (a local comment `derived_from` its id, or the
`line_note` id a store written before spans used) is not drawn; the
comment stands in its place. `render._attachSpans` owns all of this;
the explicit `grid-row` on every row of a half with spans is what
places the blocks.

The gutter **folds**, globally: folded, it is `lineno · bars` — the text
column zero wide, every block hidden, no row stretched, the bars and
dots still showing where each span is and how they nest, a mark's
tooltip its rationale. Clicking a mark unfolds it and brings that
old half's paired row so the halves stay aligned), and text running past
the hunk's last row stretches its last code row; every other row keeps
its natural height. Two spans starting on one row chain their blocks,
outermost first. The same pass gives every non-code row inside a bar — a
comment thread's row, a fold's label row — a bars cell with the bar's
segment at its depth, so a bar runs unbroken through them. The pass runs
when the half's rows change (an annotation inserted, a fold hiding rows
— a `MutationObserver`) or its size does (a `ResizeObserver`, on the
next frame); it measures once, writes once, and discards the mutation
records its own writes queue. When a fold hides a span's first row its
text hides too and the fold's label tree lists it ([[fold-region]]). A
span whose intent the reviewer has turned into a [[reviewer-comment]] (a
local comment `derived_from` its id, or the `line_note` id a store
written before spans used) loses its text block and, on one line, its
dot — the comment stands in their place — but keeps its bar, which marks
a range the comment does not. `render._attachSpans` owns all of this;
the explicit `grid-row` on every row of a half with spans is what places
the blocks.

The gutter **folds**, globally: folded, it is the bars alone at the
right edge — the text column zero wide, every block hidden, no row
stretched, the bars and dots still showing where each span is and how
they nest, a mark's tooltip its rationale. Clicking a mark unfolds it
and brings that
span's text into view; clicking the strip's empty area, or `g`,
toggles. A reading preference like the [[fold-level]], not a rung of
it: kept in localStorage (`scr-gutter-fold`), default expanded, and
Expand Down Expand Up @@ -401,7 +410,7 @@ the AST) is a projection, not a summary, and neither exists. The three
axes the ADR names are affordances at `code`, not rungs: the expand chip
hides ([[collapsible-region]]), the definition chevron folds
([[fold-region]] — a collapsed one shows its labels), the span labels
([[annotation-span]], in the centre gutter). The gutter's own fold (`g`)
([[annotation-span]], in the span gutter). The gutter's own fold (`g`)
is not a rung either: it hides the spans' text, never rows, and is a
preference kept in localStorage rather than a level carried by the
hash.
Expand Down
77 changes: 25 additions & 52 deletions semantic_code_review/viewer/assets/comments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,30 +84,15 @@ function isPromoted(annotationId: string): boolean {
return false;
}

/** Open the comment editor pre-filled with text from an LLM annotation.
* On save the new local comment carries `derived_from` set to the
* annotation's id, so the renderer can hide the source annotation. */
function openPromotionEditor(opts: {
rowEl: HTMLElement;
side: "old" | "new";
line: number;
file: string;
body: string;
/** One-click promote: save a local comment with the observation's body
* immediately and detach what the observation showed as. No mid-flight
* editor — a smell often lives in a folded hunk header where there's no
* row to anchor an inline editor on, and the user can edit/delete via
* the comment's normal edit affordance once it's saved. `derivedFrom`
* is the observation's stable id: a smell's (`<owner>:smell:<tag>`) or
* a span's, for its intent. */
function promote(opts: {
derivedFrom: string;
}): void {
_openEditor({
rowEl: opts.rowEl, side: opts.side, line: opts.line, file: opts.file,
prefillBody: opts.body, derivedFrom: opts.derivedFrom,
});
}

/** One-click promote: save a local comment with the smell's body
* immediately and detach the source pill. No mid-flight editor —
* smells often live in a folded hunk header where there's no row to
* anchor an inline editor on, and the user can edit/delete via the
* comment's normal edit affordance once it's saved. */
function promoteSmell(opts: {
smellId: string;
file: string;
side: "old" | "new";
line: number;
Expand All @@ -118,10 +103,10 @@ function promoteSmell(opts: {
const c: ReviewerComment = {
id, file: opts.file, side: opts.side, line: opts.line, body: opts.body,
created_at: now, updated_at: now,
derived_from: opts.smellId,
derived_from: opts.derivedFrom,
};
_store.save(c).then(() => {
_removeAnnotationByDerivedId(opts.smellId);
_removeAnnotationByDerivedId(opts.derivedFrom);
renderAll();
_onChange?.();
});
Expand Down Expand Up @@ -220,26 +205,18 @@ interface EditorOpts {
/** When set, the new comment is saved as a reply (in_reply_to_id pinned
* to this id). Ignored for edits of an existing comment. */
replyTo?: string | null;
/** Prefill text for the editor (new comments only — edits use existing.body). */
prefillBody?: string;
/** Stable id of the LLM annotation this comment is being promoted from.
* Persisted on the saved comment so the renderer can hide the source
* annotation. Ignored for edits of an existing comment. */
derivedFrom?: string | null;
}

function _openEditor({ rowEl, side, line, file, existing, replyTo, prefillBody, derivedFrom }: EditorOpts): void {
function _openEditor({ rowEl, side, line, file, existing, replyTo }: EditorOpts): void {
const bodyWrap = _el("div", "comment-editor-body");
const ta = _el("textarea", "comment-editor-input") as HTMLTextAreaElement;
ta.rows = 1;
ta.placeholder = existing
? "Edit comment… (Enter to save, Shift-Enter for newline, Esc to cancel)"
: derivedFrom
? "Edit and save as a comment… (Enter to save, Shift-Enter for newline, Esc to cancel)"
: replyTo
? "Write a reply… (Enter to save, Shift-Enter for newline, Esc to cancel)"
: "Write a comment… (Enter to save, Shift-Enter for newline, Esc to cancel)";
ta.value = existing ? existing.body : (prefillBody ?? "");
: replyTo
? "Write a reply… (Enter to save, Shift-Enter for newline, Esc to cancel)"
: "Write a comment… (Enter to save, Shift-Enter for newline, Esc to cancel)";
ta.value = existing ? existing.body : "";
bodyWrap.appendChild(ta);
const bar = _el("div", "comment-editor-bar");
const save = _el("button", "comment-btn comment-btn-save",
Expand Down Expand Up @@ -278,16 +255,10 @@ function _openEditor({ rowEl, side, line, file, existing, replyTo, prefillBody,
in_reply_to_id: existing
? existing.in_reply_to_id ?? null
: (replyTo ?? null),
derived_from: existing
? existing.derived_from ?? null
: (derivedFrom ?? null),
derived_from: existing ? existing.derived_from ?? null : null,
};
_store.save(c).then(() => {
close();
// If this save came from "Add as comment" on an LLM annotation,
// remove the source annotation row from the DOM so the
// observation visibly transitions into the comment.
if (c.derived_from) _removeAnnotationByDerivedId(c.derived_from);
_refreshForAnchor(rowEl, { file, side, line });
_onChange?.();
});
Expand Down Expand Up @@ -620,13 +591,16 @@ function _refreshForAnchor(anchorRowEl: HTMLElement, anchor: Anchor): void {
* source observation visibly transitions into the comment.
*
* Two shapes today: a span is its text block (`.span-text`) and marks
* (`.span-mark`) in the centre gutter, both carrying `data-span-id`;
* smells render as inline `.smell` pills with `data-smell-id`. All are
* plain elements; the gutter's placement pass notices a removed block
* on its next run. */
* (`.span-mark`) in the span gutter, both carrying `data-span-id`;
* smells render as inline `.smell` pills with `data-smell-id`. Of a
* span, the block and a dot go — the comment stands in their place —
* but a bar stays: it marks a range the comment, on one line, does
* not (the renderer draws a promoted span the same way). All are plain
* elements; the gutter's placement pass notices a removed block on its
* next run. */
function _removeAnnotationByDerivedId(derivedId: string): void {
document.querySelectorAll<HTMLElement>(
`.span-text[data-span-id="${derivedId}"], .span-mark[data-span-id="${derivedId}"]`,
`.span-text[data-span-id="${derivedId}"], .span-mark.span-dot[data-span-id="${derivedId}"]`,
).forEach((el) => el.remove());
document.querySelectorAll<HTMLElement>(
`.smell[data-smell-id="${derivedId}"]`,
Expand Down Expand Up @@ -688,8 +662,7 @@ export const Comments = {
renderAll,
getAll,
isPromoted,
openPromotionEditor,
promoteSmell,
promote,
openBlockEditor,
attachBlockThreads,
};
Loading