Update view replay captures after line numbers were removed - #2426
Closed
stephentoub wants to merge 1 commit into
Closed
Update view replay captures after line numbers were removed#2426stephentoub wants to merge 1 commit into
stephentoub wants to merge 1 commit into
Conversation
copilot-agent-runtime#13802 graduated the "no view line numbers" experiment, so the `view` tool now returns raw file content instead of prefixing every line with `N. `. The replay captures in this repo still held the numbered form, so strict snapshot matching rejected every current-runtime request that followed a `view` call, the proxy answered 500 (`proxy-error`), and the CLI retried five times before failing the test. Rewrite the 30 affected `view` tool results across 27 snapshots to the shipped raw-content form, and add a `view`-scoped result normalizer that strips a leading run of consecutively numbered lines. The normalizer keeps captures replaying against the pinned 1.0.81-11 CLI, which still emits the prefixes, and keeps newly recorded captures in the shipped form. It is scoped to `view` so tools that legitimately return numbered lines, such as `sql`, keep matching strictly. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Updates replay fixtures for raw view output while supporting older runtimes that emit line-number prefixes.
Changes:
- Removes line-number prefixes from 27 snapshots.
- Adds
view-scoped normalization and replay/capture tests. - Preserves strict normalization for other tools.
Show a summary per file
| File | Description |
|---|---|
test/harness/replayingCapiProxy.ts |
Adds view result normalization. |
test/harness/replayingCapiProxy.test.ts |
Tests capture and replay normalization. |
test/snapshots/tools/invokes_built_in_tools.yaml |
Updates raw view result. |
test/snapshots/system_message_transform/should_work_with_static_overrides_and_transforms_together.yaml |
Updates transformed-message fixture. |
test/snapshots/system_message_transform/should_invoke_transform_callbacks_with_section_content.yaml |
Updates transform-callback fixture. |
test/snapshots/system_message_transform/should_apply_transform_modifications_to_section_content.yaml |
Updates transform-modification fixture. |
test/snapshots/subagent_hooks/should_invoke_pretooluse_and_posttooluse_hooks_for_sub_agent_tool_calls.yaml |
Updates subagent view result. |
test/snapshots/session/should_send_with_file_attachment.yaml |
Updates attachment fixture. |
test/snapshots/session_config/should_use_workingdirectory_for_tool_execution.yaml |
Updates working-directory fixture. |
test/snapshots/session_config/should_apply_workingdirectory_on_session_resume.yaml |
Updates resumed-session fixture. |
test/snapshots/session_config/should_accept_message_attachments.yaml |
Updates message-attachment fixture. |
test/snapshots/permissions/should_invoke_permission_handler_for_write_operations.yaml |
Updates permission conversations. |
test/snapshots/permissions/permission_handler_for_write_operations.yaml |
Updates equivalent permission fixture. |
test/snapshots/multi_turn/should_use_tool_results_from_previous_turns.yaml |
Updates result and removes redundant conversation. |
test/snapshots/multi_turn/should_handle_file_creation_then_reading_across_turns.yaml |
Updates result and removes redundant conversation. |
test/snapshots/hooks/should_invoke_pretooluse_hook_when_model_runs_a_tool.yaml |
Updates pre-tool hook fixture. |
test/snapshots/hooks/should_invoke_posttooluse_hook_after_model_runs_a_tool.yaml |
Updates post-tool hook fixture. |
test/snapshots/hooks/should_invoke_both_pretooluse_and_posttooluse_hooks_for_single_tool_call.yaml |
Updates combined-hook fixture. |
test/snapshots/hooks/should_invoke_both_pretooluse_and_posttooluse_hooks_for_a_single_tool_call.yaml |
Updates alternate combined-hook fixture. |
test/snapshots/hooks/invoke_pre_tool_use_hook_when_model_runs_a_tool.yaml |
Updates legacy pre-hook fixture. |
test/snapshots/hooks/invoke_post_tool_use_hook_after_model_runs_a_tool.yaml |
Updates legacy post-hook fixture. |
test/snapshots/hooks/invoke_both_hooks_for_single_tool_call.yaml |
Updates legacy combined-hook fixture. |
test/snapshots/event_fidelity/should_preserve_message_order_in_getmessages_after_tool_use.yaml |
Updates message-order fixture. |
test/snapshots/event_fidelity/should_emit_tool_execution_events_with_correct_fields.yaml |
Updates tool-event fixture. |
test/snapshots/event_fidelity/should_emit_events_in_correct_order_for_tool_using_conversation.yaml |
Updates event-order fixture. |
test/snapshots/client_options/should_use_client_cwd_for_default_workingdirectory.yaml |
Updates client-CWD fixture. |
test/snapshots/builtin_tools/should_read_file_with_line_range.yaml |
Updates ranged view output. |
test/snapshots/builtin_tools/should_edit_a_file_successfully.yaml |
Updates edited-file output. |
test/snapshots/builtin_tools/should_create_a_new_file.yaml |
Updates created-file output. |
Review details
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- Files reviewed: 29/29 changed files
- Comments generated: 1
- Review effort level: Balanced
| // Lines of a CRLF file keep their carriage return after splitting on "\n". | ||
| const carriageReturn = lines[index].endsWith("\r"); | ||
| const line = carriageReturn ? lines[index].slice(0, -1) : lines[index]; | ||
| const match = /^(\d+)\.(?: (.*))?$/.exec(line); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
What
Update the
viewreplay captures to the raw-content form that current runtimes emit, and add aview-scoped tool-result normalizer so captures replay against runtimes on either side of the behavior change.viewtool results across 27 snapshots intest/snapshots/lose theirN.line-number prefixes.normalizeViewLineNumbersstrips a leading run of consecutively numbered lines fromviewresults only.Why
github/copilot-agent-runtime#13802 (
CLI: Remove line numbers from view output, merged 2026-08-26, merge commit36db7e12b91d2264060eb3f8892c25485fcefcef) graduated the winning experiment and made raw file content the unconditionalviewbehavior. That PR updated the runtime's own prompt snapshots and replay captures, but the external captures living in this repo were not updated, so they still hold the numbered form:Under the current runtime the CLI sends
Hello from multi-turn test. Strict replay matching rejects it, the replaying CAPI proxy answers500 Internal Server ErrorwithRequest-ID proxy-error, and the CLI retries five times before the session fails:Because the drift is in a tool result rather than in any provider protocol, it cascades to every backend leg of agent-runtime's
Copilot SDK C# testsworkflow —capi,anthropic-messages,openai-responses, andopenai-completions— for every test whose conversation contains aviewcall. See agent-runtime workflow runs 33072804346 (main) and 33046860889. This is upstream fixture drift in this repo; it is not caused by agent-runtime #17389.Why a normalizer as well as updated fixtures
nodejs/package-lock.jsonpins@github/copilotto1.0.81-11, published before #13802 merged, so this repo's own CI still runs a CLI that emits the prefixes. Updating the fixtures alone would fix the agent-runtime canary and break every SDK's E2E suite here. The normalizer bridges both runtimes, and it also keeps locally re-recorded captures in the shipped raw-content form.This follows the precedent set by #2013 (
Collapse built-in tool list in replay-proxy snapshot matching), which resolved the same class of runtime-version drift for the same C# canary.Matching is not loosened beyond the affected shape:
toolName: "view", so tools that legitimately return numbered lines keep matching strictly.test/snapshots/session_todos_changed/fires_session_todos_changed_and_exposes_rows_and_dependencies.yamlhas asqlresult of1. INSERT ...and is deliberately untouched.Validation
Local, Windows,
net8.0, snapshots forced read-only (GITHUB_ACTIONS=true).Reproduced first: with the fix stashed and
COPILOT_CLI_PATHpointed at a post-#13802 CLI (1.0.81-14),MultiTurnE2ETestsfails with exactly the CI signature —proxy-error,500, retried 5 times.With the fix applied:
test/harnessvitest (incl. 3 new tests)MultiTurn,BuiltinTools,Tools,Hooks,SubagentHooks,EventFidelity,SystemMessageTransformPermission,SessionConfig,Session,ClientOptions,SessionTodosChangedMultiTurnmulti_turn,builtin_toolsE2EThe 2 failures are
SessionConfigE2ETests.Vision_Enabled_Then_Disabled_Via_SetModelandVision_Disabled_Then_Enabled_Via_SetModel, which time out identically on a clean tree atmainwith the pinned CLI. They are pre-existing and unrelated: theirviewresults areViewed image file successfully.and were never numbered.New harness tests cover capture-time stripping, the non-consecutive stop condition,
sqlresults being left alone, CRLF content, and replaying both the numbered and unnumbered request forms against the same snapshot.Checklist
viewline-number shape