Skip to content

[Demo only DO NOT MERGE] feat(editor): Copilot code guides — token-level diff variant#3323

Open
Ethanlita wants to merge 2 commits into
goplus:devfrom
Ethanlita:feat/copilot-code-guide-token-diff
Open

[Demo only DO NOT MERGE] feat(editor): Copilot code guides — token-level diff variant#3323
Ethanlita wants to merge 2 commits into
goplus:devfrom
Ethanlita:feat/copilot-code-guide-token-diff

Conversation

@Ethanlita

Copy link
Copy Markdown
Contributor

Demo only — do not merge. Comparison variant of #3314.

This branch is identical to #3314 (the Copilot in-editor code guides) except for one thing: the code-change-hint red highlight — and its inline-vs-whole-line classification — diffs the edited region against the target at token/word level instead of character level.

Purpose: deploy both as preview environments so we can compare the two highlight behaviors on real edits.

Trade-offs to look for while comparing:

  • Token-level avoids character-level noise on unrelated replacements (e.g. setYpos y:0stepTo Radish).
  • But token-level is coarser: a partly-typed token doesn't match until complete (e.g. while changing 300150, typing 15 briefly shows as not-yet-matching), whereas character-level recognizes the partial prefix.

The single differing commit: feat(editor): use token-level diff for code-guide change highlight.

🤖 Generated with Claude Code

Instead of auto-applying patches or simply point to the editor area using a purple arrow, Copilot now guides the user to make the
edit themselves in the editor, so children practice writing code by hand.

A reply can render four guide elements(custom elements), each backed by an in-editor overlay:

- [NEW] code-drag-hint  — opens a drop target at a line and highlights the matching
                           API reference item, for inserting a new statement by drag
- [NEW] code-type-hint  — opens a pre-indented blank line with a "Type the code here"
                           chip and a green reference ghost, for typing a new statement
- [NEW] code-delete-hint — highlights a range in red, for deleting it
- [MODIFIED] code-change-hint     — shows the deletion (red) and addition (green) at once, for
                                     replacing existing code (inline or whole-line)

The CodeGuideController keeps a single active guide, navigates to it, and
auto-dismisses it once the suggested edit is done (whitespace-insensitive).
Completion for delete/change is region-local: an invisible Monaco decoration
tracks the target range so it stays correct even when the user edits elsewhere.
Shared logic lives in use-code-guide.ts (re-show on stream settle, reopen on
click, skip-if-already-done, blank-line scaffolding) so all elements behave
consistently.

Changes to be committed:
	new file:   node_modules/.vite/vitest/da39a3ee5e6b4b0d3255bfef95601890afd80709/results.json
	modified:   spx-gui/src/components/copilot/CopilotRound.vue
	modified:   spx-gui/src/components/copilot/context.ts
	modified:   spx-gui/src/components/copilot/copilot.ts
	modified:   spx-gui/src/components/editor/copilot/CodeChange.vue
	new file:   spx-gui/src/components/editor/copilot/CodeDeleteHint.vue
	new file:   spx-gui/src/components/editor/copilot/CodeDragHint.vue
	new file:   spx-gui/src/components/editor/copilot/CodeTypeHint.vue
	modified:   spx-gui/src/components/editor/copilot/index.ts
	new file:   spx-gui/src/components/editor/copilot/use-code-guide.ts
	modified:   spx-gui/src/components/xgo-code-editor/index.ts
	modified:   spx-gui/src/components/xgo-code-editor/ui/CodeEditorUI.vue
	modified:   spx-gui/src/components/xgo-code-editor/ui/api-reference/APIReferenceItem.vue
	modified:   spx-gui/src/components/xgo-code-editor/ui/api-reference/index.ts
	modified:   spx-gui/src/components/xgo-code-editor/ui/code-editor-ui.ts
	new file:   spx-gui/src/components/xgo-code-editor/ui/code-guide/CodeGuideUI.vue
	new file:   spx-gui/src/components/xgo-code-editor/ui/code-guide/index.test.ts
	new file:   spx-gui/src/components/xgo-code-editor/ui/code-guide/index.ts
	modified:   spx-gui/src/components/xgo-code-editor/ui/common.ts

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a new in-editor guidance system for the Copilot, adding hint components for code changes, deletions, dragging, and typing, along with the supporting controller and UI logic. The review feedback is highly constructive, pointing out several critical areas across the new hint components and helper utilities where out-of-bounds line numbers could cause Monaco API calls to throw errors. Additionally, the reviewer recommends improving the scroll-into-view behavior for highlighted API reference items and requests the removal of an accidentally committed Vitest results file under node_modules.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +79 to +81
if (textDocument == null || range.value == null) return
codeToDelete.value = textDocument.getValueInRange(range.value)
stopCapture()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

Guard against out-of-bounds ranges before calling getValueInRange. If the range goes beyond the document's line count, getValueInRange will throw an error and crash the watch effect.

    if (textDocument == null || range.value == null) return\n    const lineCount = textDocument.getValue().split(/\\r?\\n/).length\n    if (range.value.start.line < 1 || range.value.start.line > lineCount || range.value.end.line > lineCount + 1) return\n    codeToDelete.value = textDocument.getValueInRange(range.value)\n    stopCapture()

Comment on lines +186 to +187
if (textDocument == null || editorCtx == null) return
if (textDocument.getLineContent(line).trim() !== '') return // user typed into it; keep it

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

Guard against out-of-bounds line values before calling getLineContent. If the user deletes lines or edits the document while the guide is active, openedLine could be out of bounds when remove() is called, causing Monaco to throw an error.

    if (textDocument == null || editorCtx == null) return\n    const lineCount = textDocument.getValue().split(/\\r?\\n/).length\n    if (line < 1 || line > lineCount) return\n    if (textDocument.getLineContent(line).trim() !== '') return // user typed into it; keep it

@@ -0,0 +1 @@
{"version":"4.1.9","results":[[":spx-gui/src/components/xgo-code-editor/ui/code-guide/index.test.ts",{"duration":0,"failed":true}]]} No newline at end of file

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The node_modules directory and its contents (including Vitest cache/results files) should not be committed to the repository. Please remove this file and ensure node_modules/ is added to your .gitignore file.

end: { line: endLine, column: 1 }
// Guard against an out-of-range target line, so `getLineContent` below never throws.
const lineCount = textDocument.getValue().split(/\r?\n/).length
if (startLine.value < 1 || startLine.value > lineCount) return null

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Guard against out-of-bounds ranges when removeLineCount is specified. If startLine.value + removeLineCount.value exceeds lineCount + 1, the range will be invalid and could cause getValueInRange to throw or behave unexpectedly.

  if (startLine.value < 1 || startLine.value > lineCount || startLine.value + removeLineCount.value > lineCount + 1) return null

Comment on lines +71 to +72
if (codeEditor == null || isNaN(startLine.value)) return null
if (codeEditor.getTextDocument(textDocumentId.value) == null) return null

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Guard against out-of-bounds startLine values. If startLine is greater than lineCount + 1, the guide range will be invalid and won't align with the opened blank line.

  if (codeEditor == null || isNaN(startLine.value)) return null\n  const textDocument = codeEditor.getTextDocument(textDocumentId.value)\n  if (textDocument == null) return null\n  const lineCount = textDocument.getValue().split(/\\r?\\n/).length\n  if (startLine.value < 1 || startLine.value > lineCount + 1) return null

Comment on lines +72 to +73
if (codeEditor == null || isNaN(startLine.value)) return null
if (codeEditor.getTextDocument(textDocumentId.value) == null) return null

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Guard against out-of-bounds startLine values. If startLine is greater than lineCount + 1, the guide range will be invalid and won't align with the opened blank line.

  if (codeEditor == null || isNaN(startLine.value)) return null\n  const textDocument = codeEditor.getTextDocument(textDocumentId.value)\n  if (textDocument == null) return null\n  const lineCount = textDocument.getValue().split(/\\r?\\n/).length\n  if (startLine.value < 1 || startLine.value > lineCount + 1) return null

Comment on lines +28 to +30
watch(isHighlighted, (highlighted) => {
if (highlighted) itemEl.value?.scrollIntoView({ block: 'nearest', behavior: 'smooth' })
})

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Watch both isHighlighted and itemEl to ensure the element is scrolled into view even if it is highlighted before the DOM element is fully mounted or during list re-renders.

watch([isHighlighted, itemEl], ([highlighted, el]) => {\n  if (highlighted && el != null) el.scrollIntoView({ block: 'nearest', behavior: 'smooth' })\n})

@fennoai fennoai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review: Copilot code guides — token-level diff variant

Solid, well-structured feature: clean separation between the pure diff helpers (code-guide/index.ts), the composable (use-code-guide.ts), and the Monaco rendering (CodeGuideUI.vue), with good unit-test coverage and helpful JSDoc on the non-obvious decisions. Security review found no XSS/injection issues — all AI-generated content is rendered via textContent, and AI-controlled file/line/old attributes are range-guarded and only resolve against already-open in-memory documents.

Findings below (this is a COMMENT review, non-blocking). Note the PR title marks this [Demo only DO NOT MERGE] as an A/B branch against the char-level variant — most findings assume it could eventually land, so please confirm merge intent and which variant wins.

Findings without a reliable inline location

  • Committed build artifact under node_modules/node_modules/.vite/vitest/da39a3ee5e6b4b0d3255bfef95601890afd80709/results.json is added by this PR. It is a generated Vitest cache file (it even records "failed":true for index.test.ts) and should not be committed. Remove it and confirm node_modules is gitignored. (Flagged independently by quality, security, and documentation review.)

Lower-priority notes (not inlined)

  • use-code-guide.ts / CodeChange.vue repeat the getValue().split(/\r?\n/).length line-count pattern in several reactive paths; consider a shared helper backed by Monaco's O(1) getLineCount() rather than materializing the full document string per keystroke.
  • Renaming the custom element code-changecode-change-hint (CodeChange.vue, registration in index.ts) may leave previously-persisted copilot rounds containing the old <code-change> tag unrecognized on reload. Confirm persisted rounds never carry the old tag, or add a backward-compatible alias.

View job run

const target = computed(() => {
// The full range being replaced and its original text (used for the complete diff shown in chat).
// Returns null for a pure insertion (handled by the `type` guide).
const changeBase = computed<{ range: Range; oldText: string } | null>(() => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

changeBase is a reactive computed that reads textDocument.getValueInRange(range), so the red "deletion" preview shown in chat (changeBase.oldText) will drift as the user edits the target line. The previous implementation deliberately avoided this — the removed comment read "We don't want codeToDelete to change when text-document edited. So we are not using computed here." CodeDeleteHint.vue preserves the old behavior by snapshotting into a frozen ref. Consider snapshotting oldText here as well so the chat diff stays stable while the user types.

export const detailedDescription = `Display a modification based on the existing code and guide the user to make it \
themselves in the editor (there is no "Apply" button). The removed text is highlighted in red and the new code is shown \
as a green addition at the same time, so the user can see exactly what to change. In the editor the highlight is \
automatically narrowed to the part that actually differs, so unchanged text around the edit is not marked. The guide \

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This description still promises character-level narrowing ("automatically narrowed to the part that actually differs"), but patch 2/2 switched the in-editor inline highlight from diffChars to diffWordsWithSpace (token-level) in diffRemovedSpans. With token-level diffing, step 150step 180 marks the whole 150 token red rather than just 58. Update the wording to reflect token-level granularity so the model isn't given an inaccurate promise.

}

/**
* Whether changing `oldText` into `newText` is a single contiguous replacement — one changed region,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This JSDoc ("one changed region, possibly with a shared prefix/suffix") describes character-level semantics, but the function now iterates diffWordsWithSpace, so prefix/suffix and "scattered edits" are token-level. The test suite was renamed to isContiguousDiff (token level) — worth a one-line clarification here that granularity is token-level to keep the contract in sync.

// below so each keystroke doesn't re-split the whole document several times).
const lineCount = computed(() => {
void docVersion.value
return ui.activeTextDocument?.getValue().split(/\r?\n/).length ?? 0

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

getValue().split(/\r?\n/).length serializes the entire Monaco model and allocates a per-line array on every docVersion bump (i.e. each keystroke while a guide is active). Monaco's model exposes an O(1) getLineCount(); consider adding a getLineCount() accessor to the TextDocument wrapper and using it here (and in the other split-based line counts) to avoid full-document materialization in the hot path.

/** Clear the active guide. When `id` is given, only clears if it matches the active guide. */
clearGuide(id?: string): void
/** Subscribe to guide completion. Returns an unsubscribe function. */
onGuideCompleted(cb: (id: string) => void): () => void

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

onGuideCompleted is declared, documented, and implemented, but no caller consumes it — use-code-guide.ts only subscribes to onGuideCleared. It reads as an integration point that isn't wired up. Either consume it or drop it to avoid dead public API surface.

Comparison variant of the copilot code-guide: the inline change red
highlight (and its inline-vs-whole-line classification) diffs the edit
region against the target at token/word level instead of per character.

Coarser than the character-level diff (a partly-typed token doesn't match
until complete), but avoids character-level noise on unrelated snippets.
Kept on a separate branch so the two behaviors can be compared side by side
in their PR preview environments.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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.

1 participant