From 28a2806fe7b25dbdec52b5bf301e40327e3ddd77 Mon Sep 17 00:00:00 2001 From: Masamichi Hatayama Date: Wed, 29 Jul 2026 18:55:59 +0900 Subject: [PATCH 1/4] feat: include collider hierarchy paths in Collision2D capture previews (#2061) Co-authored-by: Cursor --- .../references/captured-variables.md | 1 + .../references/captured-variables.md | 1 + ...ausePointCollision2DPreviewBuilderTests.cs | 99 +++++++++++++++++++ ...ointCollision2DPreviewBuilderTests.cs.meta | 11 +++ .../Skill/references/captured-variables.md | 1 + ...cePausePointCollectionPreviewSerializer.cs | 10 ++ ...urcePausePointCollision2DPreviewBuilder.cs | 78 +++++++++++++++ ...ausePointCollision2DPreviewBuilder.cs.meta | 11 +++ 8 files changed, 212 insertions(+) create mode 100644 Assets/Tests/Editor/SourcePausePointCapture/SourcePausePointCollision2DPreviewBuilderTests.cs create mode 100644 Assets/Tests/Editor/SourcePausePointCapture/SourcePausePointCollision2DPreviewBuilderTests.cs.meta create mode 100644 Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointCollision2DPreviewBuilder.cs create mode 100644 Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointCollision2DPreviewBuilder.cs.meta diff --git a/.agents/skills/uloop-pause-point/references/captured-variables.md b/.agents/skills/uloop-pause-point/references/captured-variables.md index a4a794a8b..b5f205141 100644 --- a/.agents/skills/uloop-pause-point/references/captured-variables.md +++ b/.agents/skills/uloop-pause-point/references/captured-variables.md @@ -21,6 +21,7 @@ Read this before interpreting unexpected, missing, or truncated captured values, - Nested previews stop at `MaxCollectionPreviewDepth` (2 levels) below each captured variable: past that, an object or collection renders as type-name-only text instead of expanding — a type name where you expected contents means you hit this cap, not a bug. The budget is counted per captured variable, so reaching a value through `this` costs one extra level compared to reading it as a direct local: `this.CurrentPiece.Origin` bottoms out as a type name, while a `dropped` local holding the same piece expands to `{Kind, RotationState, Origin: {X, Y}}`. When the value you need sits too deep, pick a pause point line where it is a direct local or parameter — as its own top-level entry it starts with a fresh full budget. Primitive leaves (numbers, strings, booleans, and any type that overrides `ToString()`) always render regardless of depth; only nested objects and collections get cut off. - A value's `Value` string is not always its plain `ToString()`. A materialized collection (`List`, arrays, dictionaries, ...) previews as a shallow JSON array/object instead of the default type-name text. A custom struct/class whose declared type does not override `ToString()` previews the same way — a shallow JSON object of its fields — so you do not need to add a temporary `ToString()` override just to see its contents. A type that does override `ToString()` keeps using that result unchanged. Either kind of preview is capped by depth, element count, and length like any other captured value; the element-count cap (default 10) and the preview's character budget both scale with `enable-pause-point --max-preview-elements` (1–1000). Raising it scales the character budget proportionally, so each element keeps the same ~100-character share it has at the default — plenty for numeric or boolean cells, but individually long elements can still be clipped by the scaled budget. The enable response echoes the effective `MaxPreviewElements`. +- A captured `Collision2D` is previewed as `{"Collider":{"Name":...,"UnityObjectPath":...},"OtherCollider":{...},"RelativeVelocity":...,"ContactCount":...}` — read `UnityObjectPath` to identify both colliding objects without an extra `execute-dynamic-code` round-trip. Each of `Collider` / `OtherCollider` is either that object form or the string `"(none)"` when the collider is null or destroyed. - A multidimensional array (`int[,]`, `int[,,]`, ...) previews as `{"Shape":"Int32[2,3]","TotalElements":6,"Elements":[...]}` instead of a bare JSON array, since `Elements` alone would flatten every rank in row-major order with no way to tell it apart from an empty or 1D collection; a `T[]` or jagged `T[][]` array is unaffected and still previews as a plain JSON array. - `CapturedVariablesTruncated=true` means at least one value was clipped to the length cap or the variable-count cap stopped enumeration; clipped values are still present up to the cap. diff --git a/.claude/skills/uloop-pause-point/references/captured-variables.md b/.claude/skills/uloop-pause-point/references/captured-variables.md index a4a794a8b..b5f205141 100644 --- a/.claude/skills/uloop-pause-point/references/captured-variables.md +++ b/.claude/skills/uloop-pause-point/references/captured-variables.md @@ -21,6 +21,7 @@ Read this before interpreting unexpected, missing, or truncated captured values, - Nested previews stop at `MaxCollectionPreviewDepth` (2 levels) below each captured variable: past that, an object or collection renders as type-name-only text instead of expanding — a type name where you expected contents means you hit this cap, not a bug. The budget is counted per captured variable, so reaching a value through `this` costs one extra level compared to reading it as a direct local: `this.CurrentPiece.Origin` bottoms out as a type name, while a `dropped` local holding the same piece expands to `{Kind, RotationState, Origin: {X, Y}}`. When the value you need sits too deep, pick a pause point line where it is a direct local or parameter — as its own top-level entry it starts with a fresh full budget. Primitive leaves (numbers, strings, booleans, and any type that overrides `ToString()`) always render regardless of depth; only nested objects and collections get cut off. - A value's `Value` string is not always its plain `ToString()`. A materialized collection (`List`, arrays, dictionaries, ...) previews as a shallow JSON array/object instead of the default type-name text. A custom struct/class whose declared type does not override `ToString()` previews the same way — a shallow JSON object of its fields — so you do not need to add a temporary `ToString()` override just to see its contents. A type that does override `ToString()` keeps using that result unchanged. Either kind of preview is capped by depth, element count, and length like any other captured value; the element-count cap (default 10) and the preview's character budget both scale with `enable-pause-point --max-preview-elements` (1–1000). Raising it scales the character budget proportionally, so each element keeps the same ~100-character share it has at the default — plenty for numeric or boolean cells, but individually long elements can still be clipped by the scaled budget. The enable response echoes the effective `MaxPreviewElements`. +- A captured `Collision2D` is previewed as `{"Collider":{"Name":...,"UnityObjectPath":...},"OtherCollider":{...},"RelativeVelocity":...,"ContactCount":...}` — read `UnityObjectPath` to identify both colliding objects without an extra `execute-dynamic-code` round-trip. Each of `Collider` / `OtherCollider` is either that object form or the string `"(none)"` when the collider is null or destroyed. - A multidimensional array (`int[,]`, `int[,,]`, ...) previews as `{"Shape":"Int32[2,3]","TotalElements":6,"Elements":[...]}` instead of a bare JSON array, since `Elements` alone would flatten every rank in row-major order with no way to tell it apart from an empty or 1D collection; a `T[]` or jagged `T[][]` array is unaffected and still previews as a plain JSON array. - `CapturedVariablesTruncated=true` means at least one value was clipped to the length cap or the variable-count cap stopped enumeration; clipped values are still present up to the cap. diff --git a/Assets/Tests/Editor/SourcePausePointCapture/SourcePausePointCollision2DPreviewBuilderTests.cs b/Assets/Tests/Editor/SourcePausePointCapture/SourcePausePointCollision2DPreviewBuilderTests.cs new file mode 100644 index 000000000..c454d803e --- /dev/null +++ b/Assets/Tests/Editor/SourcePausePointCapture/SourcePausePointCollision2DPreviewBuilderTests.cs @@ -0,0 +1,99 @@ +using System.Collections.Generic; + +using Newtonsoft.Json.Linq; + +using NUnit.Framework; + +using UnityEngine; + +using io.github.hatayama.UnityCliLoop.FirstPartyTools; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor +{ + /// + /// Verifies Collision2D capture previews expose collider hierarchy paths instead of raw IDs. + /// + [TestFixture] + public sealed class SourcePausePointCollision2DPreviewBuilderTests + { + private GameObject _rootGameObject; + private GameObject _childGameObject; + private GameObject _otherGameObject; + + [TearDown] + public void TearDown() + { + if (_childGameObject != null) + { + Object.DestroyImmediate(_childGameObject); + _childGameObject = null; + } + + if (_rootGameObject != null) + { + Object.DestroyImmediate(_rootGameObject); + _rootGameObject = null; + } + + if (_otherGameObject != null) + { + Object.DestroyImmediate(_otherGameObject); + _otherGameObject = null; + } + } + + [Test] + public void BuildPreviewToken_WithSceneColliders_IncludesHierarchyPaths() + { + // Verifies Collider/OtherCollider carry Name and UnityObjectPath in {scene}:/parent/child form. + _rootGameObject = new GameObject("Root"); + _childGameObject = new GameObject("Enemy"); + _childGameObject.transform.SetParent(_rootGameObject.transform); + BoxCollider2D collider = _childGameObject.AddComponent(); + + _otherGameObject = new GameObject("Ball"); + BoxCollider2D otherCollider = _otherGameObject.AddComponent(); + + Vector2 relativeVelocity = new Vector2(1.5f, -2f); + const int contactCount = 2; + + JToken token = SourcePausePointCollision2DPreviewBuilder.BuildPreviewToken( + collider, otherCollider, relativeVelocity, contactCount); + + Assert.That(token["Collider"]["Name"].Value(), Is.EqualTo("Enemy")); + Assert.That( + token["Collider"]["UnityObjectPath"].Value(), + Is.EqualTo($"{_childGameObject.scene.name}:/Root/Enemy")); + Assert.That(token["OtherCollider"]["Name"].Value(), Is.EqualTo("Ball")); + Assert.That( + token["OtherCollider"]["UnityObjectPath"].Value(), + Is.EqualTo($"{_otherGameObject.scene.name}:/Ball")); + Assert.That(token["RelativeVelocity"].Value(), Is.EqualTo(relativeVelocity.ToString())); + Assert.That(token["ContactCount"].Value(), Is.EqualTo(contactCount)); + } + + [Test] + public void BuildPreviewToken_WithNullColliders_RendersNone() + { + // Verifies a null (or Unity fake-null) collider previews as "(none)". + JToken token = SourcePausePointCollision2DPreviewBuilder.BuildPreviewToken( + null, null, Vector2.zero, 0); + + Assert.That(token["Collider"].Value(), Is.EqualTo("(none)")); + Assert.That(token["OtherCollider"].Value(), Is.EqualTo("(none)")); + Assert.That(token["ContactCount"].Value(), Is.EqualTo(0)); + } + + [Test] + public void TryBuildToken_WithNonCollision2D_ReturnsFalse() + { + // Verifies non-Collision2D values leave the Collision2D special-case path. + List value = new List { 1, 2, 3 }; + + bool built = SourcePausePointCollision2DPreviewBuilder.TryBuildToken(value, out JToken token); + + Assert.That(built, Is.False); + Assert.That(token, Is.Null); + } + } +} diff --git a/Assets/Tests/Editor/SourcePausePointCapture/SourcePausePointCollision2DPreviewBuilderTests.cs.meta b/Assets/Tests/Editor/SourcePausePointCapture/SourcePausePointCollision2DPreviewBuilderTests.cs.meta new file mode 100644 index 000000000..c83fa6e0e --- /dev/null +++ b/Assets/Tests/Editor/SourcePausePointCapture/SourcePausePointCollision2DPreviewBuilderTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 637e602e82c094ef3a3c1e1cf8c879c7 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/references/captured-variables.md b/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/references/captured-variables.md index a4a794a8b..b5f205141 100644 --- a/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/references/captured-variables.md +++ b/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/references/captured-variables.md @@ -21,6 +21,7 @@ Read this before interpreting unexpected, missing, or truncated captured values, - Nested previews stop at `MaxCollectionPreviewDepth` (2 levels) below each captured variable: past that, an object or collection renders as type-name-only text instead of expanding — a type name where you expected contents means you hit this cap, not a bug. The budget is counted per captured variable, so reaching a value through `this` costs one extra level compared to reading it as a direct local: `this.CurrentPiece.Origin` bottoms out as a type name, while a `dropped` local holding the same piece expands to `{Kind, RotationState, Origin: {X, Y}}`. When the value you need sits too deep, pick a pause point line where it is a direct local or parameter — as its own top-level entry it starts with a fresh full budget. Primitive leaves (numbers, strings, booleans, and any type that overrides `ToString()`) always render regardless of depth; only nested objects and collections get cut off. - A value's `Value` string is not always its plain `ToString()`. A materialized collection (`List`, arrays, dictionaries, ...) previews as a shallow JSON array/object instead of the default type-name text. A custom struct/class whose declared type does not override `ToString()` previews the same way — a shallow JSON object of its fields — so you do not need to add a temporary `ToString()` override just to see its contents. A type that does override `ToString()` keeps using that result unchanged. Either kind of preview is capped by depth, element count, and length like any other captured value; the element-count cap (default 10) and the preview's character budget both scale with `enable-pause-point --max-preview-elements` (1–1000). Raising it scales the character budget proportionally, so each element keeps the same ~100-character share it has at the default — plenty for numeric or boolean cells, but individually long elements can still be clipped by the scaled budget. The enable response echoes the effective `MaxPreviewElements`. +- A captured `Collision2D` is previewed as `{"Collider":{"Name":...,"UnityObjectPath":...},"OtherCollider":{...},"RelativeVelocity":...,"ContactCount":...}` — read `UnityObjectPath` to identify both colliding objects without an extra `execute-dynamic-code` round-trip. Each of `Collider` / `OtherCollider` is either that object form or the string `"(none)"` when the collider is null or destroyed. - A multidimensional array (`int[,]`, `int[,,]`, ...) previews as `{"Shape":"Int32[2,3]","TotalElements":6,"Elements":[...]}` instead of a bare JSON array, since `Elements` alone would flatten every rank in row-major order with no way to tell it apart from an empty or 1D collection; a `T[]` or jagged `T[][]` array is unaffected and still previews as a plain JSON array. - `CapturedVariablesTruncated=true` means at least one value was clipped to the length cap or the variable-count cap stopped enumeration; clipped values are still present up to the cap. diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointCollectionPreviewSerializer.cs b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointCollectionPreviewSerializer.cs index 7ed9f2bb3..4c1f1ae28 100644 --- a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointCollectionPreviewSerializer.cs +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointCollectionPreviewSerializer.cs @@ -95,6 +95,16 @@ private static JToken BuildToken( return JValue.CreateNull(); } + // Why: Collision2D's internal fields are raw instance IDs; prefer the property-based + // preview that exposes collider hierarchy paths when main-thread Classify is available. + // Why remainingDepth: without this gate a nested Collision2D would bypass the same + // MaxCollectionPreviewDepth cutoff that BuildObjectFieldsToken already enforces. + if (remainingDepth > 0 + && SourcePausePointCollision2DPreviewBuilder.TryBuildToken(value, out JToken collision2DToken)) + { + return collision2DToken; + } + if (value is UnityEngine.Object unityObject) { return new JValue(FormatUnityObjectElement(unityObject)); diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointCollision2DPreviewBuilder.cs b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointCollision2DPreviewBuilder.cs new file mode 100644 index 000000000..2afcdd107 --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointCollision2DPreviewBuilder.cs @@ -0,0 +1,78 @@ +using Newtonsoft.Json.Linq; + +using UnityEngine; + +using io.github.hatayama.UnityCliLoop.ToolContracts; + +namespace io.github.hatayama.UnityCliLoop.FirstPartyTools +{ + /// + /// Builds a Collision2D capture preview that exposes collider hierarchy paths via properties. + /// Why: raw internal fields are instance IDs (and their type changes across Unity versions — + /// int on 2022.3, EntityId later), so they cannot identify either colliding object without an + /// extra execute-dynamic-code round-trip; property access yields the live Collider2D handles. + /// + internal static class SourcePausePointCollision2DPreviewBuilder + { + private const string NoneColliderPreview = "(none)"; + + /// + /// Tries to build a Collision2D preview token from a captured value. + /// Why: Classify uses AssetDatabase and must run on the main thread; off-thread callers + /// fall back to the generic field-token path instead. + /// + public static bool TryBuildToken(object value, out JToken token) + { + token = null; + if (value is not Collision2D collision) + { + return false; + } + + if (!MainThreadSwitcher.IsMainThread) + { + return false; + } + + token = BuildPreviewToken( + collision.collider, + collision.otherCollider, + collision.relativeVelocity, + collision.contactCount); + return true; + } + + /// + /// Builds the Collision2D preview JObject from already-extracted property values. + /// Why: EditMode tests cannot construct a real Collision2D (physics simulation only), so + /// the preview shape is unit-tested through this builder rather than TryBuildToken. + /// + internal static JToken BuildPreviewToken( + Collider2D collider, Collider2D otherCollider, Vector2 relativeVelocity, int contactCount) + { + return new JObject + { + ["Collider"] = FormatCollider(collider), + ["OtherCollider"] = FormatCollider(otherCollider), + ["RelativeVelocity"] = relativeVelocity.ToString(), + ["ContactCount"] = contactCount + }; + } + + private static JToken FormatCollider(Collider2D collider) + { + // Why: Unity's overloaded == covers destroyed/"fake null" references that a plain + // ReferenceEquals check would miss. + if (collider == null) + { + return new JValue(NoneColliderPreview); + } + + return new JObject + { + ["Name"] = collider.name, + ["UnityObjectPath"] = SourcePausePointUnityObjectClassifier.Classify(collider).Path + }; + } + } +} diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointCollision2DPreviewBuilder.cs.meta b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointCollision2DPreviewBuilder.cs.meta new file mode 100644 index 000000000..7eff17927 --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointCollision2DPreviewBuilder.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 40b1abe7ca5284f629e2cffd5c273227 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: From 4bcdb22ad5a0ecea39edf6f82e170643ae9569f9 Mon Sep 17 00:00:00 2001 From: Masamichi Hatayama Date: Wed, 29 Jul 2026 19:17:25 +0900 Subject: [PATCH 2/4] feat: serialize watch values with the captured-variable preview serializer (#2062) Co-authored-by: Cursor --- .../references/watch-expressions.md | 2 +- .../references/watch-expressions.md | 2 +- .../WatchHistoryResponseSerializationTests.cs | 99 +++++++++++++++++++ ...hHistoryResponseSerializationTests.cs.meta | 11 +++ .../Editor/WatchResponseContractTests.cs | 1 + .../WatchValueFreezeHintEvaluatorTests.cs | 49 +++++++-- .../Skill/references/watch-expressions.md | 2 +- .../PausePoint/AssemblyInfo.cs | 2 + .../SourcePausePointVariableFormatter.cs | 4 +- ...LILoop.FirstPartyTools.Watch.Editor.asmdef | 3 +- .../FirstPartyTools/Watch/WatchTools.cs | 40 +++++++- .../Watch/WatchValueFreezeHintEvaluator.cs | 25 ++++- .../internal/projectrunner/watch_types.go | 1 + tests/contracts/watch_response_contract.json | 1 + 14 files changed, 222 insertions(+), 20 deletions(-) create mode 100644 Assets/Tests/Editor/WatchHistoryResponseSerializationTests.cs create mode 100644 Assets/Tests/Editor/WatchHistoryResponseSerializationTests.cs.meta diff --git a/.agents/skills/uloop-pause-point/references/watch-expressions.md b/.agents/skills/uloop-pause-point/references/watch-expressions.md index c03b2cf21..babbedf68 100644 --- a/.agents/skills/uloop-pause-point/references/watch-expressions.md +++ b/.agents/skills/uloop-pause-point/references/watch-expressions.md @@ -13,7 +13,7 @@ uloop get-watch-values --id "speed" Because a watch only re-evaluates on a changed, paused frame, a value that looks stuck across several reads usually means no new paused frame has occurred — most often the linked pause point has not been hit again (a marker on a conditional line freezes after its first hit; see Line Placement in SKILL.md). `get-watch-values` surfaces this as a non-empty `ValueFrozenHint` on the entry once the last few evaluations came back identical; treat it as a prompt to re-trigger the code path, not as proof the value cannot legitimately stay the same. -The expression may use `UloopPausePoint.TryGetCapturedValue("name")` to inspect the latest raw pause-point capture while paused. Each history entry includes the frame and either a stringified value or an explicit error type and message. A throwing expression is recorded as an error and does not stop the Editor update loop. `--max-history` accepts 1 through 100 and drops the oldest entries after the limit. +The expression may use `UloopPausePoint.TryGetCapturedValue("name")` to inspect the latest raw pause-point capture while paused. Each history entry includes the frame and either a stringified value or an explicit error type and message. Watch values are serialized with the same preview rules as pause point `CapturedVariables`: collections become compact JSON previews (e.g. `[0,1,2]`), and types with a custom `ToString()` keep their `ToString()` form. Previews share the capture-side caps (10 elements, 1024 characters); a clipped value sets `Truncated: true` on the history entry, and the freeze hint on truncated previews warns that changes beyond the caps are invisible. A throwing expression is recorded as an error and does not stop the Editor update loop. `--max-history` accepts 1 through 100 and drops the oldest entries after the limit. ## Lifetime diff --git a/.claude/skills/uloop-pause-point/references/watch-expressions.md b/.claude/skills/uloop-pause-point/references/watch-expressions.md index c03b2cf21..babbedf68 100644 --- a/.claude/skills/uloop-pause-point/references/watch-expressions.md +++ b/.claude/skills/uloop-pause-point/references/watch-expressions.md @@ -13,7 +13,7 @@ uloop get-watch-values --id "speed" Because a watch only re-evaluates on a changed, paused frame, a value that looks stuck across several reads usually means no new paused frame has occurred — most often the linked pause point has not been hit again (a marker on a conditional line freezes after its first hit; see Line Placement in SKILL.md). `get-watch-values` surfaces this as a non-empty `ValueFrozenHint` on the entry once the last few evaluations came back identical; treat it as a prompt to re-trigger the code path, not as proof the value cannot legitimately stay the same. -The expression may use `UloopPausePoint.TryGetCapturedValue("name")` to inspect the latest raw pause-point capture while paused. Each history entry includes the frame and either a stringified value or an explicit error type and message. A throwing expression is recorded as an error and does not stop the Editor update loop. `--max-history` accepts 1 through 100 and drops the oldest entries after the limit. +The expression may use `UloopPausePoint.TryGetCapturedValue("name")` to inspect the latest raw pause-point capture while paused. Each history entry includes the frame and either a stringified value or an explicit error type and message. Watch values are serialized with the same preview rules as pause point `CapturedVariables`: collections become compact JSON previews (e.g. `[0,1,2]`), and types with a custom `ToString()` keep their `ToString()` form. Previews share the capture-side caps (10 elements, 1024 characters); a clipped value sets `Truncated: true` on the history entry, and the freeze hint on truncated previews warns that changes beyond the caps are invisible. A throwing expression is recorded as an error and does not stop the Editor update loop. `--max-history` accepts 1 through 100 and drops the oldest entries after the limit. ## Lifetime diff --git a/Assets/Tests/Editor/WatchHistoryResponseSerializationTests.cs b/Assets/Tests/Editor/WatchHistoryResponseSerializationTests.cs new file mode 100644 index 000000000..004280dc2 --- /dev/null +++ b/Assets/Tests/Editor/WatchHistoryResponseSerializationTests.cs @@ -0,0 +1,99 @@ +using System; +using System.Collections.Generic; + +using NUnit.Framework; + +using UnityEngine; + +using io.github.hatayama.UnityCliLoop.FirstPartyTools; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor +{ + /// + /// Verifies watch history values use the same preview serializer as CapturedVariables. + /// + [TestFixture] + public sealed class WatchHistoryResponseSerializationTests + { + [Test] + public void FromEntry_WithListOfInts_UsesCompactJsonPreview() + { + // Verifies materialized int lists preview as compact JSON instead of type-name ToString. + List value = new List { 0, 1, 2 }; + WatchExpressionHistoryEntry entry = CreateSuccessfulEntry(value); + + WatchHistoryResponse response = WatchHistoryResponse.FromEntry(entry); + + Assert.That(response.Success, Is.True); + Assert.That(response.Value, Is.EqualTo("[0,1,2]")); + Assert.That(response.Truncated, Is.False); + } + + [Test] + public void FromEntry_WithListOfVector2Int_QuotesElementToStringValues() + { + // Verifies Vector2Int list elements keep ToString form inside a JSON string array. + List value = new List + { + new Vector2Int(9, 3), + new Vector2Int(9, 2) + }; + WatchExpressionHistoryEntry entry = CreateSuccessfulEntry(value); + + WatchHistoryResponse response = WatchHistoryResponse.FromEntry(entry); + + Assert.That(response.Success, Is.True); + Assert.That(response.Value, Is.EqualTo("[\"(9, 3)\",\"(9, 2)\"]")); + Assert.That(response.Truncated, Is.False); + } + + [Test] + public void FromEntry_WithVector3_KeepsCustomToStringForm() + { + // Verifies types with a custom ToString keep that form instead of a field JSON preview. + Vector3 value = new Vector3(1f, 2f, 3f); + WatchExpressionHistoryEntry entry = CreateSuccessfulEntry(value); + + WatchHistoryResponse response = WatchHistoryResponse.FromEntry(entry); + + Assert.That(response.Success, Is.True); + Assert.That(response.Value, Is.EqualTo(value.ToString())); + Assert.That(response.Truncated, Is.False); + } + + [Test] + public void FromEntry_WithNullValue_ReturnsNullLiteral() + { + // Verifies a successful null evaluation still stringifies as the literal "null". + WatchExpressionHistoryEntry entry = CreateSuccessfulEntry(null); + + WatchHistoryResponse response = WatchHistoryResponse.FromEntry(entry); + + Assert.That(response.Success, Is.True); + Assert.That(response.Value, Is.EqualTo("null")); + Assert.That(response.Truncated, Is.False); + } + + [Test] + public void FromEntry_WithListExceedingElementCap_TruncatesAndSetsTruncated() + { + // Verifies a 15-element list keeps only the first 10 preview elements and sets Truncated. + List value = new List { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14 }; + WatchExpressionHistoryEntry entry = CreateSuccessfulEntry(value); + + WatchHistoryResponse response = WatchHistoryResponse.FromEntry(entry); + + Assert.That(response.Success, Is.True); + Assert.That(response.Value, Is.EqualTo("[0,1,2,3,4,5,6,7,8,9]")); + Assert.That(response.Truncated, Is.True); + } + + private static WatchExpressionHistoryEntry CreateSuccessfulEntry(object value) + { + return new WatchExpressionHistoryEntry( + frameCount: 1, + evaluatedAtUtc: DateTime.UtcNow, + result: WatchEvaluationResult.SuccessResult(value)); + } + } +} diff --git a/Assets/Tests/Editor/WatchHistoryResponseSerializationTests.cs.meta b/Assets/Tests/Editor/WatchHistoryResponseSerializationTests.cs.meta new file mode 100644 index 000000000..f1be8173a --- /dev/null +++ b/Assets/Tests/Editor/WatchHistoryResponseSerializationTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 7ba8016e441e04a118003a01bf452381 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/WatchResponseContractTests.cs b/Assets/Tests/Editor/WatchResponseContractTests.cs index 678c4d4f3..de6fc9dc5 100644 --- a/Assets/Tests/Editor/WatchResponseContractTests.cs +++ b/Assets/Tests/Editor/WatchResponseContractTests.cs @@ -49,6 +49,7 @@ public void WatchResponse_WhenSerialized_MatchesSharedContractFieldShape() EvaluatedAtUtc = "2026-06-03T00:00:01.0000000Z", Success = true, Value = "3", + Truncated = false, ErrorTypeName = "", ErrorMessage = "" } diff --git a/Assets/Tests/Editor/WatchValueFreezeHintEvaluatorTests.cs b/Assets/Tests/Editor/WatchValueFreezeHintEvaluatorTests.cs index 53846f2fe..46f626086 100644 --- a/Assets/Tests/Editor/WatchValueFreezeHintEvaluatorTests.cs +++ b/Assets/Tests/Editor/WatchValueFreezeHintEvaluatorTests.cs @@ -63,11 +63,11 @@ public void EvaluateFreezeHint_WhenARecentEvaluationFailed_ReturnsEmpty() { // Tests that evaluation errors are not mistaken for a frozen value; a failure is a // distinct problem the freeze hint should not paper over. - List history = new() + List history = new List { - CreateEntry("1", success: true), - CreateEntry("1", success: true), - CreateEntry(string.Empty, success: false) + CreateEntry("1", success: true, truncated: false), + CreateEntry("1", success: true, truncated: false), + CreateEntry(string.Empty, success: false, truncated: false) }; string hint = WatchValueFreezeHintEvaluator.EvaluateFreezeHint(history); @@ -84,23 +84,56 @@ public void EvaluateFreezeHint_WhenHistoryIsNull_ReturnsEmpty() Assert.That(hint, Is.Empty); } + [Test] + public void EvaluateFreezeHint_WhenFrozenAndRecentEntryIsTruncated_AppendsTruncationNote() + { + // Verifies truncated identical previews warn that cap-hidden changes are invisible. + List history = new List + { + CreateEntry("1", success: true, truncated: false), + CreateEntry("1", success: true, truncated: true), + CreateEntry("1", success: true, truncated: false) + }; + + string hint = WatchValueFreezeHintEvaluator.EvaluateFreezeHint(history); + + Assert.That(hint, Is.Not.Empty); + Assert.That( + hint, + Does.Contain( + "Note: the compared values are truncated previews - changes beyond the element or length cap are invisible to this comparison.")); + } + + [Test] + public void EvaluateFreezeHint_WhenFrozenWithoutTruncation_DoesNotAppendTruncationNote() + { + // Verifies the truncation caveat is omitted when every compared preview is complete. + List history = CreateHistory("1", "1", "1"); + + string hint = WatchValueFreezeHintEvaluator.EvaluateFreezeHint(history); + + Assert.That(hint, Is.Not.Empty); + Assert.That(hint, Does.Not.Contain("truncated previews")); + } + private static List CreateHistory(params string[] values) { - List history = new(); + List history = new List(); foreach (string value in values) { - history.Add(CreateEntry(value, success: true)); + history.Add(CreateEntry(value, success: true, truncated: false)); } return history; } - private static WatchHistoryResponse CreateEntry(string value, bool success) + private static WatchHistoryResponse CreateEntry(string value, bool success, bool truncated) { return new WatchHistoryResponse { Success = success, - Value = value + Value = value, + Truncated = truncated }; } } diff --git a/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/references/watch-expressions.md b/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/references/watch-expressions.md index c03b2cf21..babbedf68 100644 --- a/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/references/watch-expressions.md +++ b/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/references/watch-expressions.md @@ -13,7 +13,7 @@ uloop get-watch-values --id "speed" Because a watch only re-evaluates on a changed, paused frame, a value that looks stuck across several reads usually means no new paused frame has occurred — most often the linked pause point has not been hit again (a marker on a conditional line freezes after its first hit; see Line Placement in SKILL.md). `get-watch-values` surfaces this as a non-empty `ValueFrozenHint` on the entry once the last few evaluations came back identical; treat it as a prompt to re-trigger the code path, not as proof the value cannot legitimately stay the same. -The expression may use `UloopPausePoint.TryGetCapturedValue("name")` to inspect the latest raw pause-point capture while paused. Each history entry includes the frame and either a stringified value or an explicit error type and message. A throwing expression is recorded as an error and does not stop the Editor update loop. `--max-history` accepts 1 through 100 and drops the oldest entries after the limit. +The expression may use `UloopPausePoint.TryGetCapturedValue("name")` to inspect the latest raw pause-point capture while paused. Each history entry includes the frame and either a stringified value or an explicit error type and message. Watch values are serialized with the same preview rules as pause point `CapturedVariables`: collections become compact JSON previews (e.g. `[0,1,2]`), and types with a custom `ToString()` keep their `ToString()` form. Previews share the capture-side caps (10 elements, 1024 characters); a clipped value sets `Truncated: true` on the history entry, and the freeze hint on truncated previews warns that changes beyond the caps are invisible. A throwing expression is recorded as an error and does not stop the Editor update loop. `--max-history` accepts 1 through 100 and drops the oldest entries after the limit. ## Lifetime diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/AssemblyInfo.cs b/Packages/src/Editor/FirstPartyTools/PausePoint/AssemblyInfo.cs index eaac1fe9a..6a64b0c53 100644 --- a/Packages/src/Editor/FirstPartyTools/PausePoint/AssemblyInfo.cs +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/AssemblyInfo.cs @@ -1,6 +1,8 @@ using System.Runtime.CompilerServices; [assembly: InternalsVisibleTo("UnityCLILoop.FirstPartyTools.Editor")] +// Watch reuses the captured-variable preview serializer so get-watch-values matches CapturedVariables. +[assembly: InternalsVisibleTo("UnityCLILoop.FirstPartyTools.Watch.Editor")] [assembly: InternalsVisibleTo("UnityCLILoop.Tests.Editor.SourcePausePointResolver")] [assembly: InternalsVisibleTo("UnityCLILoop.Tests.Editor.SourcePausePointCapture")] [assembly: InternalsVisibleTo("UnityCLILoop.Tests.Editor.SourcePausePointPatcher")] diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointVariableFormatter.cs b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointVariableFormatter.cs index e336b200d..a5c2353af 100644 --- a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointVariableFormatter.cs +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointVariableFormatter.cs @@ -111,7 +111,9 @@ private static UloopCapturedVariable FormatUnityObjectVariable( classification.Kind, classification.Path, classification.InstanceId, truncated: false); } - private static string ApplyValueLengthCap(string value, int maxLength, ref bool truncated) + // Why internal: Watch reuses this cap so get-watch-values previews match CapturedVariables + // length limits instead of duplicating the clipping rule. + internal static string ApplyValueLengthCap(string value, int maxLength, ref bool truncated) { if (value.Length <= maxLength) { diff --git a/Packages/src/Editor/FirstPartyTools/Watch/UnityCLILoop.FirstPartyTools.Watch.Editor.asmdef b/Packages/src/Editor/FirstPartyTools/Watch/UnityCLILoop.FirstPartyTools.Watch.Editor.asmdef index 2ec3602cb..cd627f590 100644 --- a/Packages/src/Editor/FirstPartyTools/Watch/UnityCLILoop.FirstPartyTools.Watch.Editor.asmdef +++ b/Packages/src/Editor/FirstPartyTools/Watch/UnityCLILoop.FirstPartyTools.Watch.Editor.asmdef @@ -3,7 +3,8 @@ "rootNamespace": "io.github.hatayama.UnityCliLoop.FirstPartyTools", "references": [ "GUID:fc3fd32eddbee40e39c2d76dc184957b", - "GUID:afe86dd49995e46baa33e099a4d2ee1d" + "GUID:afe86dd49995e46baa33e099a4d2ee1d", + "GUID:94d8abc693f543a691a4645a5ff42e5c" ], "includePlatforms": [ "Editor" diff --git a/Packages/src/Editor/FirstPartyTools/Watch/WatchTools.cs b/Packages/src/Editor/FirstPartyTools/Watch/WatchTools.cs index 6be5bc7b5..b0c54bccc 100644 --- a/Packages/src/Editor/FirstPartyTools/Watch/WatchTools.cs +++ b/Packages/src/Editor/FirstPartyTools/Watch/WatchTools.cs @@ -96,24 +96,58 @@ public sealed class WatchHistoryResponse public string EvaluatedAtUtc { get; set; } = string.Empty; public bool Success { get; set; } public string Value { get; set; } = string.Empty; + public bool Truncated { get; set; } public string ErrorTypeName { get; set; } = string.Empty; public string ErrorMessage { get; set; } = string.Empty; internal static WatchHistoryResponse FromEntry(WatchExpressionHistoryEntry entry) { WatchEvaluationResult result = entry.Result; + (string value, bool truncated) = result.Success + ? FormatSuccessfulValue(result.Value) + : (string.Empty, false); return new WatchHistoryResponse { FrameCount = entry.FrameCount, EvaluatedAtUtc = entry.EvaluatedAtUtc.ToString("O"), Success = result.Success, - Value = result.Success - ? result.Value == null ? "null" : result.Value.ToString() - : string.Empty, + Value = value, + Truncated = truncated, ErrorTypeName = result.ErrorTypeName, ErrorMessage = result.ErrorMessage }; } + + // Why: watch Value used plain ToString(), so collections collapsed to type names and + // looked frozen even when contents changed; reuse CapturedVariables preview rules, + // including element/length caps so freeze-hint comparison is not silently blind. + private static (string Value, bool Truncated) FormatSuccessfulValue(object value) + { + if (value == null) + { + return ("null", false); + } + + bool truncated = false; + if (SourcePausePointCollectionPreviewSerializer.TrySerialize( + value, + SourcePausePointConstants.MaxCollectionPreviewElementCount, + ref truncated, + out string preview)) + { + // Why: capture always applies MaxCollectionPreviewValueLength; without it watch + // previews can grow unbounded across --max-history entries. + string cappedPreview = SourcePausePointVariableFormatter.ApplyValueLengthCap( + preview, + SourcePausePointConstants.MaxCollectionPreviewValueLength, + ref truncated); + return (cappedPreview, truncated); + } + + // Why: ToString fallback keeps pre-preview scalar behavior; capping it would change + // ordinary watch outputs outside this PR's serializer-unification scope. + return (value.ToString(), false); + } } /// diff --git a/Packages/src/Editor/FirstPartyTools/Watch/WatchValueFreezeHintEvaluator.cs b/Packages/src/Editor/FirstPartyTools/Watch/WatchValueFreezeHintEvaluator.cs index 04fd10a08..403dbb8fc 100644 --- a/Packages/src/Editor/FirstPartyTools/Watch/WatchValueFreezeHintEvaluator.cs +++ b/Packages/src/Editor/FirstPartyTools/Watch/WatchValueFreezeHintEvaluator.cs @@ -16,7 +16,13 @@ internal static class WatchValueFreezeHintEvaluator "Value has not changed across the last {0} evaluations. Watch values only refresh " + "when the Editor is paused on a changed frame, so confirm the linked pause point has " + "been hit again if you expect this value to be different (a marker on a conditional " + - "line freezes after its first hit)."; + "line freezes after its first hit). A custom ToString() that omits changing fields " + + "can also make a changing value appear frozen; in that case watch a more specific " + + "field or property instead."; + + private const string TruncatedPreviewFreezeHintSuffix = + " Note: the compared values are truncated previews - changes beyond the element or " + + "length cap are invisible to this comparison."; public static string EvaluateFreezeHint(IReadOnlyList history) { @@ -35,9 +41,20 @@ public static string EvaluateFreezeHint(IReadOnlyList hist string firstValue = recent[0].Value; bool allIdentical = recent.All(entry => entry.Value == firstValue); - return allIdentical - ? string.Format(FreezeHintMessageFormat, MinIdenticalEvaluationsForHint) - : string.Empty; + if (!allIdentical) + { + return string.Empty; + } + + string hint = string.Format(FreezeHintMessageFormat, MinIdenticalEvaluationsForHint); + // Why: truncated previews can stay identical while elements beyond the cap change, + // so the freeze hint would otherwise blame a missed pause-point hit incorrectly. + if (recent.Any(entry => entry.Truncated)) + { + return hint + TruncatedPreviewFreezeHintSuffix; + } + + return hint; } } } diff --git a/cli/project-runner/internal/projectrunner/watch_types.go b/cli/project-runner/internal/projectrunner/watch_types.go index 44f54851f..5f8d76012 100644 --- a/cli/project-runner/internal/projectrunner/watch_types.go +++ b/cli/project-runner/internal/projectrunner/watch_types.go @@ -26,6 +26,7 @@ type watchHistoryResponse struct { EvaluatedAtUtc string `json:"EvaluatedAtUtc"` Success bool `json:"Success"` Value string `json:"Value"` + Truncated bool `json:"Truncated"` ErrorTypeName string `json:"ErrorTypeName"` ErrorMessage string `json:"ErrorMessage"` } diff --git a/tests/contracts/watch_response_contract.json b/tests/contracts/watch_response_contract.json index 972d35210..9a1f5f213 100644 --- a/tests/contracts/watch_response_contract.json +++ b/tests/contracts/watch_response_contract.json @@ -19,6 +19,7 @@ "EvaluatedAtUtc": "2026-06-03T00:00:01.0000000Z", "Success": true, "Value": "3", + "Truncated": false, "ErrorTypeName": "", "ErrorMessage": "" } From f1469dbe34deb693a3db2fae843556045c2ae674 Mon Sep 17 00:00:00 2001 From: Masamichi Hatayama Date: Wed, 29 Jul 2026 19:44:57 +0900 Subject: [PATCH 3/4] feat: replace unknown-command list with closest-match suggestions (#2063) Co-authored-by: Cursor --- cli/common/errors/command_suggestions.go | 90 +++++++++++++++++++ cli/common/errors/command_suggestions_test.go | 70 +++++++++++++++ cli/common/errors/error_envelope.go | 2 +- cli/common/errors/error_envelope_test.go | 36 ++++++-- cli/dispatcher/shared-inputs-stamp.json | 2 +- cli/project-runner/shared-inputs-stamp.json | 2 +- 6 files changed, 192 insertions(+), 10 deletions(-) create mode 100644 cli/common/errors/command_suggestions.go create mode 100644 cli/common/errors/command_suggestions_test.go diff --git a/cli/common/errors/command_suggestions.go b/cli/common/errors/command_suggestions.go new file mode 100644 index 000000000..f2cbac5bb --- /dev/null +++ b/cli/common/errors/command_suggestions.go @@ -0,0 +1,90 @@ +package clierrors + +import "sort" + +const maxCommandSuggestions = 5 + +type commandSuggestionCandidate struct { + distance int + name string +} + +// suggestCommands returns the closest available command names for an unknown command. +// Why: listing every available command wastes tokens; a short ranked suggestion list is enough +// to recover from typos like "compil" -> "compile". +func suggestCommands(command string, availableCommands []string) []string { + if len(availableCommands) == 0 { + return []string{} + } + + candidates := make([]commandSuggestionCandidate, 0, len(availableCommands)) + for _, availableCommand := range availableCommands { + candidates = append(candidates, commandSuggestionCandidate{ + distance: levenshteinDistance(command, availableCommand), + name: availableCommand, + }) + } + + sort.Slice(candidates, func(left int, right int) bool { + if candidates[left].distance != candidates[right].distance { + return candidates[left].distance < candidates[right].distance + } + return candidates[left].name < candidates[right].name + }) + + limit := maxCommandSuggestions + if len(candidates) < limit { + limit = len(candidates) + } + + suggestions := make([]string, 0, limit) + for index := 0; index < limit; index++ { + suggestions = append(suggestions, candidates[index].name) + } + return suggestions +} + +// levenshteinDistance returns the edit distance between two strings. +func levenshteinDistance(a string, b string) int { + if a == b { + return 0 + } + if len(a) == 0 { + return len(b) + } + if len(b) == 0 { + return len(a) + } + + previous := make([]int, len(b)+1) + current := make([]int, len(b)+1) + for column := 0; column <= len(b); column++ { + previous[column] = column + } + + for row := 1; row <= len(a); row++ { + current[0] = row + for column := 1; column <= len(b); column++ { + deletionCost := previous[column] + 1 + insertionCost := current[column-1] + 1 + substitutionCost := previous[column-1] + if a[row-1] != b[column-1] { + substitutionCost++ + } + current[column] = minInt(deletionCost, insertionCost, substitutionCost) + } + previous, current = current, previous + } + + return previous[len(b)] +} + +func minInt(values ...int) int { + minimum := values[0] + for _, value := range values[1:] { + if value < minimum { + minimum = value + } + } + return minimum +} diff --git a/cli/common/errors/command_suggestions_test.go b/cli/common/errors/command_suggestions_test.go new file mode 100644 index 000000000..ece68e1b6 --- /dev/null +++ b/cli/common/errors/command_suggestions_test.go @@ -0,0 +1,70 @@ +package clierrors + +import ( + "reflect" + "testing" +) + +// Verifies suggestions are ordered by ascending Levenshtein distance. +func TestSuggestCommandsOrdersByDistance(t *testing.T) { + suggestions := suggestCommands("compil", []string{"launch", "compile", "clear-console"}) + + if len(suggestions) == 0 || suggestions[0] != "compile" { + t.Fatalf("expected compile first, got %#v", suggestions) + } +} + +// Verifies equal distances break ties by ascending command name. +func TestSuggestCommandsBreaksDistanceTiesByName(t *testing.T) { + suggestions := suggestCommands("ab", []string{"ac", "ad", "aa"}) + + expected := []string{"aa", "ac", "ad"} + if !reflect.DeepEqual(suggestions, expected) { + t.Fatalf("tie-break order mismatch: got %#v, want %#v", suggestions, expected) + } +} + +// Verifies more than five candidates are clipped to maxCommandSuggestions. +func TestSuggestCommandsCapsAtFive(t *testing.T) { + available := []string{"a", "b", "c", "d", "e", "f", "g"} + suggestions := suggestCommands("z", available) + + if len(suggestions) != maxCommandSuggestions { + t.Fatalf("expected %d suggestions, got %#v", maxCommandSuggestions, suggestions) + } +} + +// Verifies fewer than five candidates returns the full sorted list. +func TestSuggestCommandsReturnsAllWhenFewerThanCap(t *testing.T) { + available := []string{"compile", "launch"} + suggestions := suggestCommands("compil", available) + + if len(suggestions) != 2 { + t.Fatalf("expected both candidates, got %#v", suggestions) + } + if suggestions[0] != "compile" { + t.Fatalf("expected compile first, got %#v", suggestions) + } +} + +// Verifies an empty available-command list yields an empty suggestion list. +func TestSuggestCommandsReturnsEmptyForEmptyAvailableList(t *testing.T) { + suggestions := suggestCommands("compile", nil) + + if len(suggestions) != 0 { + t.Fatalf("expected empty suggestions, got %#v", suggestions) + } +} + +// Verifies identical strings have distance zero and single-char edits distance one. +func TestLevenshteinDistanceBasicCases(t *testing.T) { + if distance := levenshteinDistance("compile", "compile"); distance != 0 { + t.Fatalf("identical distance: got %d", distance) + } + if distance := levenshteinDistance("compil", "compile"); distance != 1 { + t.Fatalf("one-edit distance: got %d", distance) + } + if distance := levenshteinDistance("", "abc"); distance != 3 { + t.Fatalf("empty-to-string distance: got %d", distance) + } +} diff --git a/cli/common/errors/error_envelope.go b/cli/common/errors/error_envelope.go index 986187a2a..29d66f454 100644 --- a/cli/common/errors/error_envelope.go +++ b/cli/common/errors/error_envelope.go @@ -272,7 +272,7 @@ func UnknownCommandError(command string, availableCommands []string, context Err "Run `uloop sync` if the local tool cache may be stale.", }, Details: map[string]any{ - "AvailableCommands": availableCommands, + "SuggestedCommands": suggestCommands(command, availableCommands), }, } } diff --git a/cli/common/errors/error_envelope_test.go b/cli/common/errors/error_envelope_test.go index f46f75172..638f6ae81 100644 --- a/cli/common/errors/error_envelope_test.go +++ b/cli/common/errors/error_envelope_test.go @@ -469,22 +469,44 @@ func TestWriteToolFailureClassifiesAcceptedResponseTimeout(t *testing.T) { } } -func TestUnknownCommandErrorIncludesAvailableCommands(t *testing.T) { +// Verifies unknown-command details expose at most five closest suggestions, with typos ranked first. +func TestUnknownCommandErrorIncludesSuggestedCommands(t *testing.T) { + available := []string{ + "clear-console", + "compile", + "control-play-mode", + "execute-dynamic-code", + "find-game-objects", + "focus-window", + "get-hierarchy", + "get-logs", + "launch", + "list", + "run-tests", + "screenshot", + "sync", + } cliErr := UnknownCommandError( - "missing", - []string{"launch", "compile"}, + "compil", + available, ErrorContext{ProjectRoot: "/tmp/MyProject"}, ) if cliErr.ErrorCode != ErrorCodeUnknownCommand { t.Fatalf("error code mismatch: %#v", cliErr) } - available, ok := cliErr.Details["AvailableCommands"].([]string) + suggested, ok := cliErr.Details["SuggestedCommands"].([]string) if !ok { - t.Fatalf("available commands missing: %#v", cliErr.Details) + t.Fatalf("suggested commands missing: %#v", cliErr.Details) + } + if len(suggested) == 0 || len(suggested) > maxCommandSuggestions { + t.Fatalf("suggested commands length out of range: %#v", suggested) + } + if suggested[0] != "compile" { + t.Fatalf("expected compile first for typo compil, got %#v", suggested) } - if len(available) == 0 || available[len(available)-1] != "compile" { - t.Fatalf("available commands mismatch: %#v", available) + if _, hasAvailable := cliErr.Details["AvailableCommands"]; hasAvailable { + t.Fatalf("AvailableCommands should be removed: %#v", cliErr.Details) } } diff --git a/cli/dispatcher/shared-inputs-stamp.json b/cli/dispatcher/shared-inputs-stamp.json index 9a09e58f3..69d186c56 100644 --- a/cli/dispatcher/shared-inputs-stamp.json +++ b/cli/dispatcher/shared-inputs-stamp.json @@ -1,4 +1,4 @@ { "schemaVersion": 1, - "sharedInputsHash": "a94f640d4d0d70a5379786fadf7c5fb2335ca28d" + "sharedInputsHash": "df1d138a23c8d7606d29408c17393cd9bb32afa8" } diff --git a/cli/project-runner/shared-inputs-stamp.json b/cli/project-runner/shared-inputs-stamp.json index 1ef96ef63..378281969 100644 --- a/cli/project-runner/shared-inputs-stamp.json +++ b/cli/project-runner/shared-inputs-stamp.json @@ -1,4 +1,4 @@ { "schemaVersion": 1, - "sharedInputsHash": "e2a4495ac68e04f0639d86e638cf672282590684" + "sharedInputsHash": "ca5858203a19887e7d0793869b75c3af20b4a4cc" } From e1c86eae96c7eaa1d59e96bbe44097f6e79f188e Mon Sep 17 00:00:00 2001 From: Masamichi Hatayama Date: Wed, 29 Jul 2026 19:52:48 +0900 Subject: [PATCH 4/4] test: cover TriggerResult embedding in expired pause point responses (#2064) Co-authored-by: Cursor --- .agents/skills/uloop-pause-point/SKILL.md | 2 +- .claude/skills/uloop-pause-point/SKILL.md | 2 +- .../CliOnlyTools~/PausePoint/Skill/SKILL.md | 2 +- .../projectrunner/pause_point_trigger_test.go | 93 +++++++++++++++++++ 4 files changed, 96 insertions(+), 3 deletions(-) diff --git a/.agents/skills/uloop-pause-point/SKILL.md b/.agents/skills/uloop-pause-point/SKILL.md index 12d557673..156a72f42 100644 --- a/.agents/skills/uloop-pause-point/SKILL.md +++ b/.agents/skills/uloop-pause-point/SKILL.md @@ -172,7 +172,7 @@ If a `simulate-*` command instead returns a failure whose message says PlayMode ## Timeout Checks -If this command times out, the patched line was not reached while the command waited. Read `Error.Details.Hint` first: it names the most likely cause when PlayMode is not running, Unity is already paused, or the marker was enabled but never hit. A `PAUSE_POINT_EXPIRED` error means the marker's own `enable-pause-point --timeout-seconds` window (measured from enable, not from wait) ran out first — clear and re-enable the pause point using the returned `Id` and `TimeoutSeconds`. The countdown freezes while a hit holds the Editor paused; a manual pause without a hit does not stop it. +If this command times out, the patched line was not reached while the command waited. Read `Error.Details.Hint` first: it names the most likely cause when PlayMode is not running, Unity is already paused, or the marker was enabled but never hit. A `PAUSE_POINT_EXPIRED` error means the marker's own `enable-pause-point --timeout-seconds` window (measured from enable, not from wait) ran out first — clear and re-enable the pause point using the returned `Id` and `TimeoutSeconds`. When `--trigger` was passed, the expired envelope also carries `Error.Details.TriggerResult` (with `Completed: false` and no `Error` field when the trigger's outcome was still unknown at expiry). The countdown freezes while a hit holds the Editor paused; a manual pause without a hit does not stop it. Use `uloop pause-point-status --id "Assets/Scripts/Enemy.cs:42"` only when you need to confirm the marker is armed or inspect the current hit state. diff --git a/.claude/skills/uloop-pause-point/SKILL.md b/.claude/skills/uloop-pause-point/SKILL.md index 12d557673..156a72f42 100644 --- a/.claude/skills/uloop-pause-point/SKILL.md +++ b/.claude/skills/uloop-pause-point/SKILL.md @@ -172,7 +172,7 @@ If a `simulate-*` command instead returns a failure whose message says PlayMode ## Timeout Checks -If this command times out, the patched line was not reached while the command waited. Read `Error.Details.Hint` first: it names the most likely cause when PlayMode is not running, Unity is already paused, or the marker was enabled but never hit. A `PAUSE_POINT_EXPIRED` error means the marker's own `enable-pause-point --timeout-seconds` window (measured from enable, not from wait) ran out first — clear and re-enable the pause point using the returned `Id` and `TimeoutSeconds`. The countdown freezes while a hit holds the Editor paused; a manual pause without a hit does not stop it. +If this command times out, the patched line was not reached while the command waited. Read `Error.Details.Hint` first: it names the most likely cause when PlayMode is not running, Unity is already paused, or the marker was enabled but never hit. A `PAUSE_POINT_EXPIRED` error means the marker's own `enable-pause-point --timeout-seconds` window (measured from enable, not from wait) ran out first — clear and re-enable the pause point using the returned `Id` and `TimeoutSeconds`. When `--trigger` was passed, the expired envelope also carries `Error.Details.TriggerResult` (with `Completed: false` and no `Error` field when the trigger's outcome was still unknown at expiry). The countdown freezes while a hit holds the Editor paused; a manual pause without a hit does not stop it. Use `uloop pause-point-status --id "Assets/Scripts/Enemy.cs:42"` only when you need to confirm the marker is armed or inspect the current hit state. diff --git a/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/SKILL.md b/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/SKILL.md index 12d557673..156a72f42 100644 --- a/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/SKILL.md +++ b/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/SKILL.md @@ -172,7 +172,7 @@ If a `simulate-*` command instead returns a failure whose message says PlayMode ## Timeout Checks -If this command times out, the patched line was not reached while the command waited. Read `Error.Details.Hint` first: it names the most likely cause when PlayMode is not running, Unity is already paused, or the marker was enabled but never hit. A `PAUSE_POINT_EXPIRED` error means the marker's own `enable-pause-point --timeout-seconds` window (measured from enable, not from wait) ran out first — clear and re-enable the pause point using the returned `Id` and `TimeoutSeconds`. The countdown freezes while a hit holds the Editor paused; a manual pause without a hit does not stop it. +If this command times out, the patched line was not reached while the command waited. Read `Error.Details.Hint` first: it names the most likely cause when PlayMode is not running, Unity is already paused, or the marker was enabled but never hit. A `PAUSE_POINT_EXPIRED` error means the marker's own `enable-pause-point --timeout-seconds` window (measured from enable, not from wait) ran out first — clear and re-enable the pause point using the returned `Id` and `TimeoutSeconds`. When `--trigger` was passed, the expired envelope also carries `Error.Details.TriggerResult` (with `Completed: false` and no `Error` field when the trigger's outcome was still unknown at expiry). The countdown freezes while a hit holds the Editor paused; a manual pause without a hit does not stop it. Use `uloop pause-point-status --id "Assets/Scripts/Enemy.cs:42"` only when you need to confirm the marker is armed or inspect the current hit state. diff --git a/cli/project-runner/internal/projectrunner/pause_point_trigger_test.go b/cli/project-runner/internal/projectrunner/pause_point_trigger_test.go index dbcedc62a..6b5cd82d9 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_trigger_test.go +++ b/cli/project-runner/internal/projectrunner/pause_point_trigger_test.go @@ -9,6 +9,7 @@ import ( "testing" "time" + clierrors "github.com/hatayama/unity-cli-loop/common/errors" "github.com/hatayama/unity-cli-loop/common/unityipc" ) @@ -465,6 +466,98 @@ func TestRunWaitForPausePointEmbedsTriggerResultOnTimeout(t *testing.T) { } } +// Verifies a --trigger result is embedded in the expired error envelope's Details after the +// trigger was actually dispatched (arm confirmed Enabled first; later polls return Expired). +func TestRunWaitForPausePointEmbedsTriggerResultOnExpired(t *testing.T) { + originalQuery := queryPausePointStatus + originalClear := clearPausePointStatus + originalDispatch := dispatchPausePointTriggerCommand + originalPoll := pausePointStatusPoll + pausePointStatusPoll = time.Millisecond + defer func() { + queryPausePointStatus = originalQuery + clearPausePointStatus = originalClear + dispatchPausePointTriggerCommand = originalDispatch + pausePointStatusPoll = originalPoll + }() + + // Why stateful: an unconditional Expired on the first query makes arm confirmation fail, so + // the trigger is never dispatched and only a "not dispatched" placeholder TriggerResult appears. + statusQueryCount := 0 + queryPausePointStatus = func( + ctx context.Context, + connection unityipc.Connection, + id string, + ) (pausePointStatusResponse, error) { + statusQueryCount++ + if statusQueryCount == 1 { + return pausePointStatusResponse{ + Id: id, + Status: pausePointStatusEnabled, + IsEnabled: true, + EditorState: pausePointEditorState{IsPlaying: true, CapturedAt: "Current"}, + }, nil + } + return pausePointStatusResponse{ + Id: id, + Status: pausePointStatusExpired, + Expired: true, + IsEnabled: false, + EditorState: pausePointEditorState{IsPlaying: true, CapturedAt: "Current"}, + }, nil + } + clearPausePointStatus = func( + ctx context.Context, + connection unityipc.Connection, + id string, + ) (pausePointStatusResponse, error) { + return pausePointStatusResponse{Id: id, Status: pausePointStatusCleared}, nil + } + dispatchPausePointTriggerCommand = func( + ctx context.Context, + connection unityipc.Connection, + command string, + commandArgs []string, + startPath string, + stdout io.Writer, + stderr io.Writer, + ) int { + _, _ = stdout.Write([]byte(`{"Success":true}`)) + return 0 + } + + var stdout bytes.Buffer + var stderr bytes.Buffer + code := runWaitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ + id: "jump", + timeoutSeconds: 1, + timeout: 50 * time.Millisecond, + triggerCommand: "simulate-keyboard", + triggerArgs: []string{"--action", "Press"}, + }, &stdout, &stderr) + + if code != 1 { + t.Fatalf("expected expired failure, got %d with stdout %s stderr %s", code, stdout.String(), stderr.String()) + } + envelope := parsePausePointErrorEnvelope(t, stderr.Bytes()) + if envelope.Error.ErrorCode != clierrors.ErrorCodePausePointExpired { + t.Fatalf("error code mismatch: %#v", envelope.Error) + } + triggerResult, ok := envelope.Error.Details["TriggerResult"].(map[string]any) + if !ok { + t.Fatalf("TriggerResult detail missing or wrong shape: %#v", envelope.Error.Details) + } + if triggerResult["Command"] != "simulate-keyboard --action Press" { + t.Fatalf("TriggerResult command mismatch: %#v", triggerResult) + } + if triggerResult["Completed"] != true { + t.Fatalf("TriggerResult should report Completed=true after dispatch: %#v", triggerResult) + } + if errorText, _ := triggerResult["Error"].(string); errorText != "" { + t.Fatalf("TriggerResult Error should be empty after a successful dispatch: %#v", triggerResult) + } +} + // Verifies await-pause-point parses --trigger into the command/args pair and rejects a value that // targets another pause-point wait. func TestParseWaitForPausePointOptionsParsesTriggerFlag(t *testing.T) {