Skip to content

vscode: align editor Comments-API review-comment write to append (markerAppendLine) for parity with the preview composer (#1107 follow-up) #1122

Closed

Description

@amrmelsayed

Problem

After PR #1121 (PIR #1107) shipped the inline composer for adding review comments from the markdown preview, the preview-composer path and the editor Comments-API path now write new comments in opposite stack orders against the same on-disk format.

  • Preview composer (packages/vscode/src/markdown-preview/preview-provider.ts): uses markerAppendLine — new comment lands AFTER the existing thread (newest-last). Matches the composer's visual position (composer renders below the existing markers).
  • Editor Comments-API (packages/vscode/src/comments/plan-review.ts:171): still uses markerInsertionLine — new comment lands BEFORE the existing thread (newest-first / prepended).

Two authoring surfaces, one on-disk format, divergent UX. The canvas renderer + parseReviewMarkers are agnostic to insert position; both surfaces produce valid on-disk state. But a reviewer adding a follow-up comment to an existing thread will see different stack semantics depending on which surface they used, which is confusing and breaks the "newest-last in reading order" mental model the composer path established.

Surfaced honestly in the PR #1121 disposition by the PIR builder; tracked separately to keep the parent issue's scope clean per feedback_spin_off_cross_cutting_ideas.

Source

packages/vscode/src/comments/plan-review.ts:171:

edit.insert(
  thread.uri,
  new vscode.Position(markerInsertionLine(line), 0),
  commentLine + '\n',
);

The import at packages/vscode/src/comments/plan-review.ts:24 is markerInsertionLine from @cluesmith/codev-core/review-markers.

Fix

Swap markerInsertionLine → markerAppendLine at both sites (import + use). The helper already exists in core (packages/core/src/review-markers.ts:71-78, added by PR #1121), with three regression tests covering the load-bearing cases (empty thread, stacked markers, end-of-file edge).

Both signatures take (text: string, annotatedLine: number) for append vs (line: number) for insertion — markerAppendLine needs the full document text to scan for existing markers. The editor Comments-API site has the document available via vscode.workspace.openTextDocument(thread.uri), so the text is one line of additional code: const text = document.getText(); before the existing edit.insert(...).

Acceptance criteria

  • plan-review.ts:171 writes the new marker at markerAppendLine(text, line) instead of markerInsertionLine(line).
  • Reply to an existing thread via the editor Comments-API surface lands the new comment AFTER the existing markers, newest-last in render order (parity with the preview composer's behaviour).
  • Regression test covering the editor-side append path — likely a small extension to plan-review.ts's test surface (or a new test file if there isn't one for this codepath today), asserting that posting a reply to a thread that already has two markers writes the new marker at line position N+1 (past the run), not at position 1 (top of the run).
  • No regression to the first-comment case (a block with zero existing markers): the append position equals the insertion position there, so the first-marker write is unchanged.
  • No change to markerAppendLine or markerInsertionLine in core — they're already correct.

Out of scope

Protocol

BUGFIX. Mechanical one-line behaviour change with the underlying helper already tested in core. The fix is import { markerAppendLine } from '@cluesmith/codev-core/review-markers' + one call site swap + one regression test.

Related

Activity

  1. self-assigned this
    on Jul 1, 2026
  2. amrmelsayed commented on Jul 1, 2026

    @amrmelsayed
    CollaboratorAuthor

    On it! Working on a fix now.

  3. added 3 commits that reference this issue on Jul 1, 2026
  4. added 2 commits that reference this issue on Jul 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

area/vscodeArea: VS Code extension

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions