mirror of
https://github.com/microsoft/vscode.git
synced 2026-07-06 23:06:20 +01:00
Ulugbek Abdullaev
a3e359ab0c
* nes-datagen: add cursor-jump (NCLP) task
Extend nes-datagen with a next-cursor-line prediction task alongside
the existing xtab path. Detects the user's next intentional cursor
move after the request bookmark and emits a training sample with the
production cursor-prediction prompt + the observed jump as the
expected response.
Three sub-modes via --sample-task:
- cursor-same-file: a jump farther than N lines from cursor at
request time
- cursor-cross-file: focus/selection on a different file
- cursor-both: either of the above
Reuses the production cursor-prediction prompt by capturing it via
the telemetry builder and a no-op fetcher; the cross-file target
line is resolved from a request-time content snapshot + post-request
replay so previously-opened targets get a correct line number
instead of being silently labelled :0.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* nes-datagen: replace SAMPLE_TASK_VALUES tuple with a string enum
Convert the string-union + as-const tuple to a proper NesDatagenSampleTask
string enum. CLI surface is string-enum members keep theunchanged
kebab-case wire values ('xtab', 'cursor-same-file', ...). All consumers
(dispatch, fixtures, response metadata typing) updated to reference enum
members instead of string literals.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* nes-datagen: lower default --same-file-jump-min-above to 2
Upward cursor jumps (back to a definition, an import, etc.) are
typically tighter than downward jumps after the user has been
writing. Lower the default threshold to match.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* nes-datagen: rename NCLP to cursor-jump throughout
Drop the NCLP abbreviation in favor of the more descriptive
'cursor-jump' name already used in the production xtab provider.
cursorJumpPromptStep,
cursorJumpResponseStep), the capture request ids,
and all surrounding doc comments / test descriptions.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* nes-datagen: build documentIndexMapping from whole recording
path map from only the
pre-request slice and then re-walked the post-request slice to
backfill any documentEncountered entries that arrived later. Pass the
whole recording into documentIndexMapping instead so the helper sees
every document the user touched in a single pass; the backfill loop is
gone.
splitRecordingAtRequestTime now also returns the full entries array so
both callers can reuse it without re-deriving it from altAction.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* nes-datagen: use shared Result type in cursor-jump detectors
Drop the bespoke { ok, value | reason } discriminated union in
detectJump.ts and reuse the existing Result<T, E> from
src/util/common/result. JumpDetectionResult<T> is now just an alias
for Result<T, string>.
.isOk(),
.err) and the spec file accordingly.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* nes-datagen: strip raw cursor-jump prompt from emitted telemetry
cursorJumpRawMessages and cursorJumpKeptRange were added to
IStatelessNextEditTelemetry so in-process debug / datagen tooling
could read them back via getStatelessNextEditTelemetry(). However
LlmNESTelemetryBuilder.build() spreads ...this._statelessNextEditTelemetry
into the emitted payload, so those two fields would leak to telemetry
cursorJumpRawMessages can contain full prompt content (sourcesinks
code), which must never leave the process.
Destructure them out before spreading into the build() payload. They
remain readable via getStatelessNextEditTelemetry() for tooling.
Documented the privacy contract on the IStatelessNextEditTelemetry
field declarations so future edits don't forget.
Addresses copilot-pull-request-reviewer feedback on PR #320113.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* nes-datagen: fail cross-file detection when no selection lands on target
detectCrossFileJump previously returned Result.ok with toLine
undefined when only a focused event was seen for the target doc (no
selectionChanged). That left generateCrossFileResponse to drop the
sample later while the detector still reported a successful jump.
Treat focused-without-selectionChanged as a failed detection
('crossFileTargetNoSelection') so callers can skip early, and tighten
ICrossFileJump.toLine to non-undefined now that ok results always
have a usable line number. Removes the dead error path in
generateCrossFileResponse.
Adds a regression test that focused-only triggers the new error.
Addresses copilot-pull-request-reviewer feedback on PR #320113.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* nes-datagen: capture cursor-jump prompt via logContext, not telemetry
The datagen pipeline previously stashed the raw cursor-jump prompt and
keptRange on IStatelessNextEditTelemetry so cursorJumpPromptStep.ts could
read them back via LlmNESTelemetryBuilder.getStatelessNextEditTelemetry().
That leaked raw prompts into the telemetry payload (worked around by a
destructure-strip hack in LlmNESTelemetryBuilder.build()) and was
asymmetric with the xtab path, which captures via
InlineEditRequestLogContext.rawMessages.
Move the cursor-jump capture vehicle onto InlineEditRequestLogContext to
match xtab:
- Add cursorJumpRawMessages / cursorJumpKeptRange fields and
setCursorJumpPrompt(messages, keptRange) to InlineEditRequestLogContext.
- XtabNextCursorPredictor.predictNextCursorPosition now takes a logContext
parameter and writes to it directly. The xtabProvider callsite passes
the same logContext it already had in scope.
- cursorJumpPromptStep reads from logContext instead of the telemetry
builder.
- Remove cursorJumpRawMessages / cursorJumpKeptRange from
IStatelessNextEditTelemetry, plus the corresponding setter/getter on
StatelessNextEditTelemetryBuilder and the getter on
LlmNESTelemetryBuilder.
- Revert the destructure-strip hack in LlmNESTelemetryBuilder.build().
The pre-existing cursorJumpPrompt telemetry field (JSON-stringified, fed
by setCursorJumpPrompt(messages)) is intentional and unchanged.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* nes-datagen: cursor-jump ground truth is first user EDIT, not cursor landing
Selection-based detection treated peek, navigation, IDE auto-scroll, and
recursive cursor settling as if they were the user's next intended edit
location. The model's job is to predict where the user will EDIT next, so
key off the first 'changed' event after the request bookmark instead.
Same-file detector:
- Walks for the first 'changed' on the active doc; uses the first edit's
start offset to compute toLine; applies the linesAbove/linesBelow
threshold. Bails with editsAnotherFileFirst when a non-active doc is
edited first (lets the cross-file detector claim the sample in
cursor-both mode). 'selectionChanged' is no longer consulted, so the
settle-after-edit filter is gone it was a workaround for thetoo
selection-based approach.
Cross-file detector:
- Walks for the first 'changed' on a non-active doc; uses the first
edit's start offset, resolved against the target doc's snapshot
just-before applying the event. Drops focused / selectionChanged
heuristics and the crossFileTargetNoSelection error path (a focused
event without an edit no longer counts; background peek can't
pollute the dataset).
buildLineResolver: tightened i <= entryIndex to i < entryIndex so the
resolver returns the pre-edit line when entryIndex is itself a 'changed'
event. The bound is equivalent for the old selectionChanged caller.
Spec: switched ground-truth events from selChanged to changed; added
coverage for first-edit-of-multi-edit, editsAnotherFileFirst, and
active-doc-then-other-doc ordering.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* xtab: predictNextCursorPosition takes RequestTracingContext
Every other helper in xtabProvider takes RequestTracingContext (the
{ tracer, logContext, telemetry } bundle). The cursor predictor was the
odd one out, taking the three pieces as separate positional params with
the latter two that asymmetry made the new logContext-captureoptional
plumbing look more invasive than it is and forced an awkward
?.setCursorJumpPrompt chain at the use site.
Switch the predictor to take RequestTracingContext directly:
- Export RequestTracingContext from xtabProvider so the predictor can
type-import it (TS-erased to avoid the runtime circular import).
- predictNextCursorPosition signature collapses from 5 params to 3.
- Drop the optional chains; tracing.telemetry / tracing.logContext are
always present in production and the spec constructs a real bundle.
- Spec adds a createTestTracingContext helper using the cheap
InlineEditRequestLogContext / StatelessNextEditTelemetryBuilder
constructors already used by other inlineEdits specs.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* nes-datagen: rename splitRecording 'entries' field to 'wholeRecording'
Review feedback: the field on the splitRecordingAtRequestTime return
shape was named 'entries' but in context it carries the whole unsplit
recording (i.e. before slicing into prior/after parts). 'wholeRecording'
matches the comment at the consumer (documentIndexMapping callsite).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* nes-datagen: inline JumpDetectionResult<T> as Result<T, string>
Review feedback: the one-line alias was used in exactly two places in
the same file and gave nothing over the underlying Result type.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* nes-datagen: discriminated union for sample task + jump metadata
Review feedback: ISampleMetadata had 'task' + an optional 'jump' field
with toFilePath also optional. That let xtab samples accidentally carry
a jump and let cursor-cross-file samples omit toFilePath. Replace with
a discriminated union on task:
- xtab: no jump
- cursorSameFile: jump with fromLine/toLine/distance
- cursorCrossFile: jump with required toFilePath
assembleSample now takes a single SampleClassification arg, removing
the parallel task/jump parameters that callers had to keep in sync.
cursorJumpResponseStep is split into ISameFileGeneratedResponse and
ICrossFileGeneratedResponse so the generator return types map cleanly
to the union variants without a non-null assertion at the assembly
site.
DetectedJump no longer needs an assistantTask hint: the pipeline
constructs the classification directly from the response shape.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* fix dup import
* nes-datagen: address round-4 review feedback on pipeline.ts
Five review threads on pipeline. all addressed in-place.ts
- modelResponse: cursor samples were emitting an empty string for the
expected response. Populate it with the assistant content (which IS
the expected output) so downstream tooling has the gold label.
- Promise.all unbounded throws: wrap the limiter callback body in
try/catch so an unexpected exception from generateCursorPromptFromRecording
becomes a recorded per-row error instead of aborting the whole batch
via Promise.all's first-rejection semantics.
- Inline import for OffsetRange: replace the inline import('...').OffsetRange
type expression with a regular top-of-file import.
- Duplicated config-override block: both pipelines applied the same
applyConfigFile + four setConfig debounce/cache disables. Extract
into applyBatchModeConfig(configService, configs) and call from both.
- runInputPipeline parallelism + memory: add a doc comment clarifying
that this is the single-process entry point, that cursor-jump tasks
also benefit from runInputPipelineParallel (--sample-task is
propagated to workers), and that loadAndParseInput is in-memory by
design (sized per worker; use --parallelism > 1 for large inputs).
Full architectural unification of the parallel and non-parallel
paths is intentionally left as a follow- the surface area isup
large and out of scope for this PR.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* nes-datagen: add e2e tests for cursor-jump pipeline
Mirrors the existing xtab pipeline.e2e.spec.ts: drives two fixture rows
(a same-file jump and a cross-file jump) through the full
`runInputPipeline` for each `sampleTask` mode (cursor-same-file,
cursor-cross-file, cursor-both) and asserts on the JSONL output.
Coverage:
- only the matching row is emitted per mode; both rows are emitted in
cursor-both
- emitted samples carry strategy=next-cursor-line-prediction and the
correct discriminated `task` field (cursor-same-file / cursor-cross-file)
- assistant message targets the jumped-to line / file
- metadata.modelResponse mirrors the assistant content (the round-4 fix)
- --row-offset is reflected in metadata.rowIndex
Test fixtures are constructed in
`fixtures/cursorJumpFixtureData.ts` with synthesized recordings: an
explicit no-op edit + selectionChanged before the bookmark so the
cursor-prediction path's recent-edit gating is satisfied, then a
single post-request `changed` event the detector picks up.
The cursor pipeline needs a prompting strategy whose response handler
tolerates an empty stream — use `xtabUnifiedModel` in a dedicated
`cursorJumpConfig.json` (the existing patchBased02 config crashes on
empty output, which is acceptable in production but breaks the
prompt-only capture path).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Strengthen cursor-jump e2e assertions
Replace fuzzy matchers (toMatch(/25/), arrayContaining for tasks) with
exact assertions on assistant content, metadata.task, and metadata.jump.
In cursor-both, locate samples by filePath so a row→classification swap
would now be caught instead of passing.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Make cursor-jump e2e helper accept partial nesDatagen overrides
Helper previously took Partial<RunPipelineOptions>; if a caller passed
`nesDatagen`, the spread fully replaced the default block and the
configured path. Now the helper accepts a partial nesDatagen overlay
and merges field-by-field, so the row-offset test only specifies the
two fields it actually changes and there are no non-null assertions.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Add within-threshold cursor-jump negative fixture
Scenario C: cursor on line 10, post-request edit on line 12 (only 2
lines below). Default threshold is ±5 lines, so neither the same-file
nor the cross-file generator should emit a sample for this row.
Asserted in cursor-both via a dedicated 'does not emit a sample for
the within-threshold row' test, and implicitly in cursor-same-file /
cursor-cross-file (their existing count==1 assertions would fail if
the threshold guard regressed).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---------
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Ulugbek Abdullaev <ulugbekna@github.com>