-
Notifications
You must be signed in to change notification settings - Fork 682
feat:action trace feat:HookSystem #598
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: beta
Are you sure you want to change the base?
Conversation
Fixes CoplayDev#538 The System Requirements panel showed "UV Package Manager: Not Found" even when a valid UV path override was configured in Advanced Settings. Root cause: PlatformDetectorBase.DetectUv() only searched PATH with bare command names ("uvx", "uv") and never consulted PathResolverService which respects the user's override setting. Changes: - Refactor DetectUv() to use PathResolverService.GetUvxPath() which checks override path first, then system PATH, then falls back to "uvx" - Add TryValidateUvExecutable() to verify executables by running --version instead of just checking File.Exists - Prioritize PATH environment variable in EnumerateUvxCandidates() for better compatibility with official uv install scripts - Fix process output read order (ReadToEnd before WaitForExit) to prevent potential deadlocks Co-Authored-By: ChatGLM 4.7 <noreply@zhipuai.com>
- Read both stdout and stderr when validating uv/uvx executables
- Respect WaitForExit timeout return value instead of ignoring it
- Fix version parsing to handle extra tokens like "(Homebrew 2025-01-01)"
- Resolve bare commands ("uv"/"uvx") to absolute paths after validation
- Rename FindExecutableInPath to FindUvxExecutableInPath for clarity
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…s PATH augmentation Replace direct Process.Start calls with ExecPath.TryRun across all platform detectors. This change: - Fixes potential deadlocks by using async output reading - Adds proper timeout handling with process termination - Removes redundant fallback logic and simplifies version parsing - Adds Windows PATH augmentation with common uv, npm, and Python installation paths Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The version extraction logic now properly handles outputs like: - "uvx 0.9.18" -> "0.9.18" - "uvx 0.9.18 (hash date)" -> "0.9.18" - "uvx 0.9.18 extra info" -> "0.9.18" Uses Math.Min to find the first occurrence of either space or parenthesis. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add absolute path resolution in TryValidatePython and TryValidateUvWithPath for better UI display - Fix BuildAugmentedPath to avoid PATH duplication - Add comprehensive comments for version parsing logic - Ensure cross-platform consistency across all three detectors - Fix override path validation logic with clear state handling - Fix platform detector path resolution and Python version detection - Use UserProfile consistently in GetClaudeCliPath instead of Personal - All platforms now use protected BuildAugmentedPath method This change improves user experience by displaying full paths in the UI while maintaining robust fallback behavior if path resolution fails. Co-Authored-By: GLM4.7 <noreply@zhipuai.com>
- Rename TryValidateUvExecutable -> TryValidateUvxExecutable for consistency - Add cross-platform FindInPath() helper in ExecPath.cs - Remove platform-specific where/which implementations in favor of unified helper - Add Windows-specific DetectUv() override with enhanced uv/uvx detection - Add WinGet shim path support for Windows uvx installation - Update UI labels: "UV Path" -> "UVX Path" - Only show uvx path status when override is configured Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…ides and system paths
…ethods across platform detectors
… WindowsPlatformDetector
Implement a comprehensive ActionTrace system that captures, stores, and queries Unity editor events for debugging, analysis, and undo/replay capabilities. **Core Features:** - Event capture layer with hooks for Unity editor events - Context tracking with stack and timeline support - Event store with in-memory persistence and query capabilities - Semantic analysis (categorization, scoring, intent inference) - VCS integration for version control context - Editor window with UI for visualizing events - MCP tools for remote query and control **Components Added:** - Capture: ActionTraceEventEmitter, EventFilter, PropertyChangeTracker, UnityEventHooks - Context: ContextStack, ContextTimeline, OperationContext, ContextMapping - Core: EventStore, EditorEvent, EventTypes, ActionTraceSettings, GlobalIdHelper - Query: ActionTraceQuery, EventSummarizer, TransactionAggregator - Semantics: DefaultCategorizer, DefaultEventScorer, DefaultIntentInferrer - UI: ActionTraceEditorWindow with UXML/USS styling - MCP Tools: get_action_trace, get_action_trace_settings, add_action_trace_note, undo_to_sequence **Server-side:** - Python models and resources for ActionTrace - MCP tools for querying events, managing settings, and undoing to sequence Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…ility - Extract EventStore.Context.cs for context mapping management - Extract EventStore.Diagnostics.cs for memory diagnostics and dehydration - Extract EventStore.Merging.cs for event deduplication logic - Extract EventStore.Persistence.cs for save/load and domain reload survival - Add PropertyEventPayloadBuilder helper for consistent payload structure - Add PropertyFormatter helper to eliminate code duplication - Adjust ActionTraceSettings defaults (MaxEvents: 800, HotEventCount: 150) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…ctionTraceEditorWindow
… clarity and consistency
…uery improvements
This commit introduces significant improvements to the ActionTrace system:
**Layered Settings Architecture:**
- Refactor ActionTraceSettings into layered structure (Filtering, Merging, Storage, Sampling)
- Add custom Editor with foldout sections for better UX
- Move GlobalIdHelper from Core/ to Helpers/ for better organization
**Preset System:**
- Add ActionTracePreset with Standard/Verbose/Minimal/Silent presets
- Enable quick configuration switching via ApplyPreset()
- Include preset descriptions and memory estimates
**Configurable Filtering:**
- Transform EventFilter from static blacklist to rule-based system
- Support Prefix, Extension, Regex, and GameObject rule types
- Add EventFilterSettings for persistence and customization
**Stable Cross-Session IDs:**
- Use GlobalIdHelper for all GameObject/Component event TargetIds
- Use Asset:{path} format for asset events
- Ensure TargetIds remain stable across domain reloads
**Query & Analysis Enhancements:**
- Add EventQueryBuilder for fluent query API
- Add ContextCompressor for event data optimization
- Add EventStatistics for comprehensive analytics
- Enhance EventSummarizer with better grouping
**Capture Layer Improvements:**
- Add AssetChangePostprocessor for asset change tracking
- Add SamplingMiddleware for high-frequency event throttling
- Add ToolCallScope for MCP tool call tracking
- Enhance UnityEventHooks with comprehensive event coverage
**UI/UX Improvements:**
- Add sort modes (ByTimeDesc, AIFiltered) to ActionTraceEditorWindow
- Improve filter menu with "All" option
- Add tool descriptions for better AI discoverability
**Helper Classes:**
- Add GameObjectTrackingHelper for GameObject lifecycle tracking
- Add UndoReflectionHelper for Undo system introspection
- Add BuildTargetUtility for build target detection
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…al loops and enhancing GameObject tracking
…capacity - Add Range attributes to settings fields with Inspector sliders: - MinImportanceForRecording: [Range(0f, 1f)] - MergeWindowMs: [Range(0, 5000)] - TransactionWindowMs: [Range(100, 10000)] - HotEventCount: [Range(10, 1000)] - MaxEvents: [Range(100, 5000)] (already existed) - Change ContextMappings from fixed 2000 to dynamic MaxEvents × 2 - Remove redundant validation code (now enforced by Range attributes) - Fix UndoReflectionHelper.GetPreviousValue to try nested value first - Clean up StdioBridgeHost: remove ineffective CleanupZombieListeners - Add task/conversation filtering to ActionTraceViewResource - Preserve modified-object identity in SelectionPropertyTracker Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…on in ActionTrace
…leaks Replace infinite recursive delayCall patterns with EditorApplication.update in PropertyChangeTracker and SamplingMiddleware. This ensures proper cleanup on domain reload and prevents memory leaks. Changes: - PropertyChangeTracker: Replace delayCall with update + FlushCheck() - SamplingMiddleware: Replace delayCall with update directly - Remove redundant helper methods (use UndoReflectionHelper/PropertyFormatter) - Clean up unused using statements Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
@whatevertogo Sorry for the late reply on my end - aside from personal things to attend to I needed to think about this one a bit more. I think the video will help a lot. It's not immediately clear to me how this resource actually helps AI tools when building something. It looks awesome, but what's the real world use case? For e.g. recently @dsarno added a tool specifically for interacting with scriptable objects. The capability existed, but it was inefficient and prone to error. So by adding the tool, the AI tool got over a hurdle with Unity MCP that it couldn't do before. In this case, what was it that your AI tool COULD NOT DO, and how does this actually make it accomplish more. As a side note, you've been contributing a lot and I really really appreciate it. I think all of the core maintainers do. Even with disagreements (which we have amongst ourselves too), we hope that you continue being a part of our community and contributing. You're awesome ⭐ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR introduces a comprehensive event tracking and monitoring infrastructure for AI-assisted Unity development, consisting of two foundational systems:
Changes:
- Implements HookSystem: A general-purpose event dispatch infrastructure that abstracts Unity's native callbacks into a clean, thread-safe subscription interface with exception isolation
- Implements ActionTrace: A sophisticated event tracking system featuring event recording, semantic analysis, dehydration for memory efficiency, and AI-friendly query interfaces
- Adds MCP tools for AI interaction:
get_action_trace,get_action_trace_summary,add_action_trace_note,undo_to_sequence, andget_action_trace_settings
Reviewed changes
Copilot reviewed 76 out of 76 changed files in this pull request and generated 57 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/Hooks/DESIGN.md | Comprehensive design documentation for Hook system architecture |
| docs/Hooks/CONTRIBUTING.md | Contributing guide for extending the Hook system |
| docs/ActionTrace/DESIGN.md | Design philosophy and architectural decisions for ActionTrace |
| docs/ActionTrace/CONTRIBUTING.md | Contributing guide for ActionTrace extensions |
| Server/src/services/tools/*.py | Python MCP tool implementations for ActionTrace queries |
| Server/src/services/resources/action_trace.py | Python resource endpoints for ActionTrace |
| Server/src/models/action_trace.py | Pydantic models for ActionTrace data validation |
| MCPForUnity/Editor/Windows/ActionTraceEditorWindow.* | Unity Editor window for visualizing action traces |
| MCPForUnity/Editor/Tools/*Tool.cs | C# MCP tool handlers for ActionTrace operations |
| MCPForUnity/Editor/Resources/ActionTrace/*.cs | C# resource handlers and query infrastructure |
| MCPForUnity/Editor/Hooks/*.cs | Core Hook system implementation with event registry |
| MCPForUnity/Editor/Helpers/*.cs | Helper utilities for GlobalID resolution and build targets |
| MCPForUnity/Editor/ActionTrace/**/*.cs | ActionTrace subsystems (Semantics, Integration, Sources) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
MCPForUnity/Editor/ActionTrace/Analysis/Summarization/EventSummarizer.cs
Show resolved
Hide resolved
MCPForUnity/Editor/ActionTrace/Analysis/Summarization/EventSummarizer.cs
Show resolved
Hide resolved
| from typing import Annotated, Any, Literal, Optional | ||
|
|
||
| from fastmcp import Context | ||
| from mcp.types import ToolAnnotations |
Copilot
AI
Jan 22, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import of 'ToolAnnotations' is not used.
| from models.action_trace import ( | ||
| EditorEvent, | ||
| EventQueryResult, | ||
| EventStatistics, | ||
| ActionTraceSettings, | ||
| ) |
Copilot
AI
Jan 22, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import of 'ActionTraceSettings' is not used.
|
|
||
| from services.registry import mcp_for_unity_tool | ||
| from services.tools import get_unity_instance_from_context | ||
| from services.tools.utils import coerce_bool |
Copilot
AI
Jan 22, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import of 'coerce_bool' is not used.
|
|
||
| Unity implementation: MCPForUnity/Editor/Tools/ActionTraceSettingsTool.cs | ||
| """ | ||
| from typing import Annotated, Any |
Copilot
AI
Jan 22, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import of 'Annotated' is not used.
| parsed = json.loads(value) | ||
| if isinstance(parsed, list): | ||
| return _coerce_int_list(parsed) # Recursively handle parsed list | ||
| except json.JSONDecodeError: |
Copilot
AI
Jan 22, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'except' clause does nothing but pass and there is no explanatory comment.
2cdeb19 to
6feadbf
Compare
… and improve property modification handling. Removed unused UndoCapture and UndoToSequenceTool files. Updated ActionTrace settings and enhanced Python tool descriptions for clarity.
|
Just re-iterataing @whatevertogo, please explain the exact use case where this feature improved what the model could do
|
https://www.youtube.com/watch?v=ykhhOa-g7PI By the way,I’m working on improving the manage_prefab implementation. |
Remove unused PropertyEventPayloadBuilder class (75 lines) that had no references in the codebase. Rename classes to match their filenames: - AssetChangePostprocessor → AssetCapture - PropertyChangeTracker → PropertyCapture - SelectionPropertyTracker → SelectionCapture This improves code discoverability and follows C# naming conventions where class names should match file names for clarity. Files removed: - Helpers/PropertyEventPayloadBuilder.cs - Helpers/PropertyEventPayloadBuilder.cs.meta
…ctionCapture - Add OnPropertiesModified event to HookRegistry for Undo property changes - Add Undo.postprocessModifications subscription in UnityEventHooks - Refactor PropertyCapture to use HookRegistry.OnPropertiesModified - Refactor SelectionCapture to use HookRegistry events - Remove redundant -= operations in UnityEventHooks constructor - Remove NotifyGameObjectDestroyed(null) call, use only Detailed version - Refactor ActionTraceEditorWindow into modular folder structure - Add EventListSection component for better separation of concerns - Fix PlayModeChanged event importance score (0.3 -> 0.5) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry @whatevertogo, your pull request is larger than the review limit of 300000 diff characters
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 73 out of 73 changed files in this pull request and generated 26 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
MCPForUnity/Editor/ActionTrace/Core/Store/EventStore.Persistence.cs
Outdated
Show resolved
Hide resolved
MCPForUnity/Editor/ActionTrace/Core/Store/EventStore.Diagnostics.cs
Outdated
Show resolved
Hide resolved
| ```csharp | ||
| public readonly struct EditorEvent | ||
| { | ||
| public readonly long Sequence; | ||
| public readonly long TimestampUnixMs; | ||
| public readonly string Type; | ||
| public readonly string Payload; // JSON | ||
| } |
Copilot
AI
Jan 24, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The design doc claims EditorEvent is a readonly struct, but the implementation in MCPForUnity/Editor/ActionTrace/Core/Models/EditorEvent.cs is a sealed class. This mismatch can confuse contributors (e.g., around mutability/GC). Update the snippet and surrounding rationale to reflect the actual type used by the codebase (or change the code to match the doc).
MCPForUnity/Editor/ActionTrace/Core/Settings/ActionTraceSettings.cs
Outdated
Show resolved
Hide resolved
| if (settings != null && !settings.Filtering.BypassImportanceFilter) | ||
| { | ||
| float importance = DefaultEventScorer.Instance.Score(@event); | ||
| if (importance <= settings.Filtering.MinImportanceForRecording) |
Copilot
AI
Jan 24, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Importance filtering rejects events when importance == MinImportanceForRecording (uses <=), but the tooltip says “below this value will not be recorded”. If the threshold is meant to be inclusive, change the comparison to importance < MinImportanceForRecording to avoid dropping borderline events.
| if (importance <= settings.Filtering.MinImportanceForRecording) | |
| if (importance < settings.Filtering.MinImportanceForRecording) |
| // Wait for process to actually exit after Kill | ||
| process.WaitForExit(1000); | ||
| } | ||
| catch { } |
Copilot
AI
Jan 24, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Poor error handling: empty catch block.
| var token = @params["min_importance"] ?? @params["minImportance"]; | ||
| if (token != null) | ||
| { | ||
| string value = token?.ToString()?.ToLower()?.Trim(); |
Copilot
AI
Jan 24, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Condition is always not null because of ... != ....
| bool inStaticMode = _uiMinImportance >= 0; | ||
|
|
||
| // Skip if nothing changed | ||
| if (noNewEvents && noSearchText && !effectiveImportanceChanged && |
Copilot
AI
Jan 24, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Condition is always false because of access to local variable effectiveImportanceChanged.
| try | ||
| { | ||
| var projectPath = System.IO.Path.GetDirectoryName(UnityEngine.Application.dataPath); | ||
| var gitPath = System.IO.Path.Combine(projectPath, ".git"); |
Copilot
AI
Jan 24, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This assignment to gitPath is useless, since its value is never read.
| if (token != null) | ||
| { | ||
| if (bool.TryParse(token.ToString(), out bool summaryOnly)) | ||
| { | ||
| return summaryOnly; | ||
| } | ||
| } |
Copilot
AI
Jan 24, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These 'if' statements can be combined.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 17
🤖 Fix all issues with AI agents
In `@MCPForUnity/Editor/ActionTrace/Capture/Capturers/AssetCapture.cs`:
- Around line 145-246: The current dedup set _processedAssetsInSession prevents
legitimate later events because entries never expire; replace the
HashSet<string> with Dictionary<string,long> storing last-processed Unix-ms,
update the add/check logic in the three places that currently call
_processedAssetsInSession.Add(...) to instead prune via
CleanupOldEntries(ttlMs), then treat an entry as "already processed" only if
(now - lastProcessed) < ttlMs and otherwise update the timestamp and allow
processing; persist/load the dictionary in SaveProcessedAssets and the
corresponding load routine so timestamps survive domain reloads and implement
CleanupOldEntries to remove aged entries (TTL configurable, e.g., 5–30 minutes).
In `@MCPForUnity/Editor/ActionTrace/Capture/Capturers/SelectionCapture.cs`:
- Around line 35-47: The static ctor in SelectionCapture registers
HookRegistry.OnSelectionChanged -> OnSelectionChanged and
HookRegistry.OnPropertiesModified -> OnPropertyModified but never unsubscribes
before domain/assembly reload; add a cleanup subscription to
UnityEditor.AssemblyReloadEvents.beforeAssemblyReload (or
AssemblyReloadEvents.beforeAssemblyReload) that calls a new private static
UnsubscribeHooks() which removes those two handlers, and update the McpLog.Debug
tag from "[SelectionPropertyTracker]" to "[SelectionCapture]" so the log
reflects the correct class name.
- Around line 162-170: The code creates an EditorEvent (evt) and calls
EventStore.Record(evt) directly; change this to apply the sampling middleware
check used by Recorder.RecordEvent(): call SamplingMiddleware.ShouldRecord(evt)
(or delegate to Recorder.RecordEvent(evt) if available) and only invoke
EventStore.Record(evt) when it returns true so rapid property changes are
throttled; ensure you reference the existing EditorEvent instance (evt) and
perform the check immediately before recording.
In `@MCPForUnity/Editor/ActionTrace/Capture/Recorder.cs`:
- Around line 45-47: Both OnComponentRemoved and OnComponentRemovedDetailed are
subscribed and both emit "ComponentRemoved" events, causing duplicates; mirror
the GameObjectDestroyed/Detailed fix by unsubscribing the non-detailed handler
and only handling the detailed event (or add a de-dup guard inside the
handlers). Concretely: remove or stop subscribing to
HookRegistry.OnComponentRemoved in Recorder.cs and ensure
HookRegistry.OnComponentRemovedDetailed is the sole source for recording
component removals (also update the corresponding handler logic that writes to
EventStore/RecordEvent and remove any duplicate recording logic in the
alternative handler paths referenced around lines 108-140).
In `@MCPForUnity/Editor/ActionTrace/Core/Settings/ActionTraceSettings.cs`:
- Around line 254-260: GetEstimatedMemoryUsage can return a negative estimate
when Storage.HotEventCount > Storage.MaxEvents; change the calculation in
GetEstimatedMemoryUsage to clamp coldEvents to zero (e.g., coldEvents =
Math.Max(0, Storage.MaxEvents - Storage.HotEventCount)) or otherwise compute
totalEvents = Math.Max(Storage.HotEventCount, Storage.MaxEvents) and derive
hot/cold from that so the returned long is never negative; update references to
Storage.HotEventCount and Storage.MaxEvents inside the GetEstimatedMemoryUsage
method accordingly.
- Around line 58-60: The tooltip for HotEventCount in ActionTraceSettings.cs is
wrong about which events are retained; update the attribute on the HotEventCount
field so it explains that the last N events (most recent/newest) retain full
payload while older events beyond that limit are dehydrated (Payload=null),
referencing the HotEventCount field in ActionTraceSettings and matching the
dehydration logic in EventStore.Diagnostics.cs which keeps payloads for the most
recent entries.
In `@MCPForUnity/Editor/ActionTrace/Integration/VCS/VcsContextProvider.cs`:
- Around line 272-277: The subprocess in FindGitExecutable uses
process.WaitForExit() without a timeout which can hang the editor; update the
code that starts the git probe (the Process created from startInfo in
VcsContextProvider.FindGitExecutable) to use a bounded wait like
process.WaitForExit(5000) (or a configurable timeout), handle the case where
WaitForExit returns false (timed out) by killing/disposing the process and
treating it as failure, and ensure you still check process.ExitCode == 0 only
after a successful wait before returning "git".
- Around line 54-58: The static constructor of VcsContextProvider currently runs
whenever the type is referenced and unconditionally assigns _currentContext via
GetInitialContext() and registers EditorApplication.update += OnUpdate, causing
unnecessary Git polling even when the feature is disabled; update the static
initialization to avoid registering the update callback unless the feature is
enabled (e.g., add a guard that checks the feature flag or a IsEnabled method
before calling EditorApplication.update) or remove the static constructor
entirely and lazily initialize _currentContext and register OnUpdate inside
GetCurrentContext() (or an explicit Initialize method) so registration only
happens when the VCS feature is active.
In `@MCPForUnity/Editor/Tools/GetActionTraceSummaryTool.cs`:
- Around line 147-149: The duration calculation assumes events are in ascending
order but EventStore.Query returns newest-first, causing negative durations;
update the calculation in GetActionTraceSummaryTool so startTimeMs =
Math.Min(events[0].TimestampUnixMs, events[events.Count - 1].TimestampUnixMs)
and endTimeMs = Math.Max(events[0].TimestampUnixMs, events[events.Count -
1].TimestampUnixMs) (or use events.Min(e => e.TimestampUnixMs) and events.Max(e
=> e.TimestampUnixMs)), then set durationMs = endTimeMs - startTimeMs; reference
the existing variables events, startTimeMs, endTimeMs, and durationMs when
applying the change.
- Around line 323-344: Event selection is wrong because EventStore.Query returns
events newest-first, so FirstOrDefault(e => e.TimestampUnixMs >= thresholdMs)
picks the newest matching event instead of the oldest at/after the threshold;
update the logic in GetActionTraceSummaryTool to pick the oldest matching event
by either: scanning for the last matching element (use LastOrDefault on
sampleEvents with the same predicate) or explicitly sort/iterate sampleEvents in
ascending TimestampUnixMs then use FirstOrDefault, and return that element's
Sequence (leave the try/catch and null fallback intact).
In
`@MCPForUnity/Editor/Windows/ActionTraceEditorWindow/ActionTraceEditorWindow.cs`:
- Around line 165-172: InitializeSettings currently mutates the shared
ActionTraceSettings.Instance.Filtering.BypassImportanceFilter which can leak if
the window crashes; instead stop writing to the global setting: introduce a
window-local flag (e.g. _bypassImportanceFilterOverride) and assign it true in
InitializeSettings (and set _previousBypassImportanceFilter only if you still
need to detect prior state), remove the direct assignment to
ActionTraceSettings.Instance.Filtering.BypassImportanceFilter, and update all
internal filtering code in this window to consult the local override (use
_bypassImportanceFilterOverride ||
ActionTraceSettings.Instance.Filtering.BypassImportanceFilter) so global state
is never mutated; remove reliance on restoring in OnDisable or keep restoration
conditional if you must preserve original behavior.
- Around line 654-658: AddImportanceBar currently calls
category.ToLowerInvariant() when building the CSS class which can NRE if
category is null; update AddImportanceBar to guard against null by deriving a
safeName (e.g., use category ?? "unknown" or string.Empty) before calling
ToLowerInvariant or skip adding the importance-bar-fill--... class when category
is null, then use that safeName when calling fill.AddToClassList so
VisualElement fill and AddToClassList never receive a null-derived string.
In
`@MCPForUnity/Editor/Windows/ActionTraceEditorWindow/ActionTraceEditorWindow.uss`:
- Around line 446-454: The .detail-scroll-view CSS rule has contradictory width
constraints (min-width: 320px vs max-width: 300px); update the rule so max-width
is >= min-width (for example set max-width to 400px or 450px) to remove the
conflict and ensure predictable layout behavior in ActionTraceEditorWindow.
In
`@MCPForUnity/Editor/Windows/ActionTraceEditorWindow/Components/EventListSection.cs`:
- Around line 290-296: SetSortMode currently ignores its parameter; add a
private field (e.g. _sortMode) to store the passed SortMode in the
EventListSection class, set _sortMode = sortMode inside SetSortMode, and update
ApplySorting to switch on _sortMode instead of always sorting by timestamp
(handle the different SortMode cases such as Timestamp, Importance, etc.). Also
update any comment references that mention _sortMode to match the new field and
ensure callers rely on SetSortMode to change sorting behavior; alternatively
remove the parameter and callers if you prefer to eliminate sort-by-mode
support.
In `@Server/src/services/tools/get_action_trace_summary.py`:
- Around line 27-33: The decorator description in `@mcp_for_unity_tool` claims a
"15min summary" and "Defaults: 15min window" but the function
get_action_trace_summary has a default time_range="1h" (also referenced on the
description lines ~40-41); make them consistent by either changing the function
parameter default time_range to "15m" (or an equivalent value used elsewhere) or
updating the decorator description to state the actual default "1h" in both
places; update the description text and any other decorator lines that mention
the default so callers and docs match the function signature.
- Around line 97-102: The code currently coerces limit via coerce_int but
doesn't enforce the documented 1–500 range, allowing invalid values; update the
block in get_action_trace_summary (the section that handles limit/coerced_limit
and params_dict) to validate and enforce the range—either clamp coerced_limit to
the inclusive range 1..500 or raise/reject when out of range—then only set
params_dict["limit"] to the validated value (e.g., validated_limit = max(1,
min(coerced_limit, 500)) before assignment) so Unity always receives a limit
within 1–500.
In `@Server/src/services/tools/get_action_trace.py`:
- Around line 64-68: The current handling of limit in get_action_trace
(variables: limit, coerced_limit, coerce_int, params_dict["limit"]) accepts any
integer; modify the logic to enforce the documented 1–1000 range by validating
coerced_limit after coerce_int returns a value and either clamp it into the
[1,1000] range or raise a clear validation error; then only assign the bounded
value to params_dict["limit"]. Ensure you perform this check where coerced_limit
is set so out-of-range negatives or huge values are prevented before being sent
downstream.
♻️ Duplicate comments (4)
MCPForUnity/Editor/ActionTrace/Integration/VCS/VcsContextProvider.cs (2)
175-176: Unused variablegitPath.
gitPathis assigned but never used inRunGitCommand. This was previously flagged.var projectPath = System.IO.Path.GetDirectoryName(UnityEngine.Application.dataPath); -var gitPath = System.IO.Path.Combine(projectPath, ".git");
203-209: Empty catch block swallows exceptions.The empty catch at line 209 silently swallows any exception from
process.Kill(). Consider logging at debug level or adding a comment explaining why it's safe to ignore.catch { } +// Kill may throw if process already exited; safe to ignoreMCPForUnity/Editor/ActionTrace/Capture/Sampling/SamplingMiddleware.cs (1)
219-228: Unused variableoldestSample.The variable
oldestSampleis assigned on line 226 but never used - the code usesremovedSamplefromTryRemoveon line 230 instead. This is dead code.♻️ Proposed fix
// Manual loop to find oldest entry (avoid LINQ allocation in hot path) string oldestKey = null; long oldestTimestamp = long.MaxValue; - PendingSample oldestSample = default; foreach (var kvp in _pendingSamples) { if (kvp.Value.TimestampMs < oldestTimestamp) { oldestTimestamp = kvp.Value.TimestampMs; oldestKey = kvp.Key; - oldestSample = kvp.Value; } }MCPForUnity/Editor/ActionTrace/Capture/Recorder.cs (1)
30-30: Field_trackingHelpercan bereadonly.Since
_trackingHelperis only assigned in the static constructor and never reassigned, it should be marked asreadonly.♻️ Proposed fix
- private static GameObjectTrackingHelper _trackingHelper; + private static readonly GameObjectTrackingHelper _trackingHelper;
🧹 Nitpick comments (17)
Server/src/services/tools/get_action_trace.py (1)
106-107: Avoid swallowing unexpected exceptions without logging.A blanket
Exceptioncatch hides root causes; at least log the traceback so failures are diagnosable.♻️ Suggested logging
+import logging + +logger = logging.getLogger(__name__) @@ except Exception as e: + logger.exception("get_action_trace failed") return {"success": False, "message": f"Python error getting action trace: {str(e)}"}MCPForUnity/Editor/ActionTrace/Helpers/PropertyModificationHelper.cs (2)
21-53: Consider caching reflection results for hot-path performance.
GetNestedValueis called frequently fromProcessModifications(seePropertyCapture.cslines 76-171) which processes every property modification. Thepath.Split('.')allocation and repeatedGetProperty/GetFieldreflection lookups on each call may cause GC pressure in high-frequency edit scenarios.For a "Chill" review, this is acceptable for now, but if profiling shows this as a bottleneck, consider caching
PropertyInfo/FieldInfolookups by type+path key.
76-83: Minor: Redundant null check pattern.Line 79 checks
result != nullthen casts again withas string. SinceGetNestedValuereturnsobject, a simpler pattern would be:- var result = GetNestedValue(modification, "currentValue.propertyPath"); - if (result != null) return result as string; + if (GetNestedValue(modification, "currentValue.propertyPath") is string current) + return current;Not a bug—just a style nitpick.
MCPForUnity/Editor/ActionTrace/Integration/VCS/VcsContextProvider.cs (1)
72-84: Thread-safety claim is overstated.The comment states "Thread-safe (called from any thread)" but lines 78-81 have a check-then-act race: two threads could both see
_currentContext == nulland redundantly callGetInitialContext(). SinceVcsContextis effectively immutable after creation and reference assignment is atomic, this won't corrupt data, but it's not truly thread-safe.Either add a lock/
Interlocked.CompareExchangeor soften the comment to "Safe for concurrent reads; initialization may run redundantly."MCPForUnity/Editor/ActionTrace/Core/Store/EventStore.Persistence.cs (1)
117-126: Inconsistent locking for_saveScheduled.
ScheduleSaveaccesses_saveScheduledunder_queryLock, butClearPendingOperationssets it tofalseoutside any lock (line 124). This inconsistency could cause races ifClearPendingOperationsis called while a save is being scheduled.Since this is typically called during shutdown, it's low risk, but for consistency:
♻️ Suggested fix
public static void ClearPendingOperations() { lock (_pendingNotifications) { _pendingNotifications.Clear(); _notifyScheduled = false; } + lock (_queryLock) + { _saveScheduled = false; + } _lastDehydratedCount = -1; // Reset dehydration optimization marker }MCPForUnity/Editor/Windows/ActionTraceEditorWindow/Components/EventListSection.cs (2)
173-259: BindListItem performs repeated Q<> queries on each bind.Lines 180-188 call
element.Q<>()for 9 different elements on every bind. For virtualized lists with frequent scrolling, this causes repeated tree traversals. Consider caching element references in a user data object attached to the root element duringMakeListItem.For a Chill review, this is acceptable if performance is adequate, but it's a known pattern for optimization.
366-406: Complex change detection logic; consider simplifying or documenting.
ShouldSkipRefreshhas intricate state tracking across 5+ variables. While the logic appears correct, it's difficult to reason about edge cases. Consider adding a brief comment block explaining the refresh-skip conditions, or refactoring into named helper methods for each condition category.MCPForUnity/Editor/Windows/ActionTraceEditorWindow/ActionTraceEditorWindow.uxml (1)
40-44: Consider consistent naming for duplicate "Clear Filters" buttons.Lines 43 and 81 define two "Clear Filters" buttons (
clear-filters-buttonandclear-filters-button-2). If they share the same handler, consider using a common CSS class for event binding rather than name-based lookups, or document why two distinct names are needed.Also applies to: 73-83
MCPForUnity/Editor/ActionTrace/Capture/Capturers/SelectionCapture.cs (1)
199-214: Duplicate code:GetGameObjectPathexists in bothSelectionCaptureandRecorder.This same method implementation appears in
Recorder.cs(lines 343-357). Consider extracting this to a shared helper class to avoid duplication and ensure consistent behavior.MCPForUnity/Editor/Windows/ActionTraceEditorWindow/ActionTraceEditorWindow.cs (1)
274-322:FilterSectionstores callbacks but lacks cleanup method.Callbacks are stored in fields (lines 274-278) and registered (lines 318-322), but there's no
UnregisterCallbacksmethod likeEventListSectionhas. While the window lifetime typically matches the section lifetime, explicitly unregistering provides cleaner resource management.MCPForUnity/Editor/Windows/ActionTraceEditorWindow/ActionTraceEditorWindow.uss (1)
204-209: Negative padding value may not behave as expected.
padding-top: -2pxon line 206 is unusual. Unity's UIToolkit USS may not support negative padding values consistently. If this is intended to adjust spacing, consider usingmargin-topwith a negative value or adjusting the layout differently.MCPForUnity/Editor/ActionTrace/Capture/Capturers/PropertyCapture.cs (1)
121-152: Prefer avoiding manualMonitor.Exit/Enterinsidelock.
This pattern is non-idiomatic and can be fragile; consider moving the flush outside the critical section with a flag/recheck to keep lock discipline straightforward.MCPForUnity/Editor/ActionTrace/Core/EventTypes.cs (1)
98-103: PromoteAINoteto a first-class constant.
Using a literal here sidesteps the checklist and can drift from other references.♻️ Suggested tweak
+ public const string AINote = "AINote"; ... - ["AINote"] = new EventMetadata + [AINote] = new EventMetadataMCPForUnity/Editor/Tools/GetActionTraceSummaryTool.cs (1)
159-161: Avoid creating a newDefaultEventScorerinstance per event.A
DefaultEventScoreris already instantiated at line 90. Reuse it inside the loop to avoid N allocations.♻️ Proposed fix
- // Count by importance (using scorer) - var scorer = new DefaultEventScorer(); - float importance = scorer.Score(evt); + // Count by importance (reuse scorer from outer scope) + float importance = scorer.Score(evt);This requires renaming the outer
scorervariable or passing it intoGenerateSummary. Alternatively, useDefaultEventScorer.Instance:- var scorer = new DefaultEventScorer(); - float importance = scorer.Score(evt); + float importance = DefaultEventScorer.Instance.Score(evt);MCPForUnity/Editor/ActionTrace/Capture/Emitter.cs (1)
459-477: Consider clarifying the distinction betweenEmitAssetDeletedoverloads.There are two
EmitAssetDeletedmethods:
EmitAssetDeleted(string assetPath)(line 286) - general asset deletionEmitAssetDeleted(string assetPath, string assetType)(line 459) - MCP tool-initiated deletion withsource = "mcp_tool"This design appears intentional, but the subtle difference (source tracking) may not be obvious to callers. Consider renaming the MCP-specific variant to
EmitAssetDeletedViaToolor adding a clearer XML doc note explaining when to use each.MCPForUnity/Editor/ActionTrace/Analysis/Query/ActionTraceQuery.cs (1)
144-151: Consider combining the if statements for consistency withProjectWithContext.
ProjectWithContextuses a combined conditional pattern (lines 220-221), while this method uses separate nested if statements. This is functionally correct but could be unified for consistency.♻️ Proposed fix
// Convert GlobalID to InstanceID and Name (with caching) (int? instanceId, string displayName) targetInfo = (null, null); - if (!string.IsNullOrEmpty(evt.TargetId)) + if (!string.IsNullOrEmpty(evt.TargetId) && + !instanceInfoCache.TryGetValue(evt.TargetId, out targetInfo)) { - if (!instanceInfoCache.TryGetValue(evt.TargetId, out targetInfo)) - { - targetInfo = GlobalIdHelper.GetInstanceInfo(evt.TargetId); - instanceInfoCache[evt.TargetId] = targetInfo; - } + targetInfo = GlobalIdHelper.GetInstanceInfo(evt.TargetId); + instanceInfoCache[evt.TargetId] = targetInfo; }MCPForUnity/Editor/Hooks/HookRegistry.cs (1)
119-139:dynamicdispatch introduces runtime overhead; consider typed alternatives for hot paths.The
InvokeSafelyhelper usesAction<dynamic>which incurs DLR (Dynamic Language Runtime) overhead on each invocation. For high-frequency events likeOnEditorUpdate, this could add measurable cost.For editor tooling this is likely acceptable, but if profiling reveals issues, consider strongly-typed overloads or cached delegate invocations.
| // L0 Deduplication: Skip if already processed in this session | ||
| // This prevents duplicate events when Unity fires OnPostprocessAllAssets | ||
| // multiple times for the same asset (creation, compilation, re-import) | ||
| if (!_processedAssetsInSession.Add(assetPath)) | ||
| continue; // Already processed, skip to prevent duplicate events | ||
|
|
||
| hasChanges = true; // Mark that we added a new entry | ||
|
|
||
| // L1 Blacklist: Skip junk assets before creating events | ||
| if (!EventFilter.ShouldTrackAsset(assetPath)) | ||
| { | ||
| // Remove from tracking if it's a junk asset (we don't want to track it) | ||
| _processedAssetsInSession.Remove(assetPath); | ||
| continue; | ||
| } | ||
|
|
||
| string targetId = $"Asset:{assetPath}"; | ||
| string assetType = GetAssetType(assetPath); | ||
|
|
||
| var payload = new Dictionary<string, object> | ||
| { | ||
| ["path"] = assetPath, | ||
| ["extension"] = System.IO.Path.GetExtension(assetPath), | ||
| ["asset_type"] = assetType | ||
| }; | ||
|
|
||
| // Mutually exclusive event classification (prevents duplicate events) | ||
| if (IsNewlyCreatedAsset(assetPath)) | ||
| { | ||
| // Priority 1: Newly created assets (first-time existence) | ||
| RecordEvent(EventTypes.AssetCreated, targetId, payload); | ||
| } | ||
| else if (ShouldTrackModification(assetPath)) | ||
| { | ||
| // Priority 2: Existing assets with trackable modification types | ||
| // Covers: re-imports, content changes, settings updates | ||
| RecordEvent(EventTypes.AssetModified, targetId, payload); | ||
| } | ||
| else | ||
| { | ||
| // Priority 3: Generic imports (fallback for untracked types) | ||
| RecordEvent(EventTypes.AssetImported, targetId, payload); | ||
| } | ||
| } | ||
|
|
||
| // ========== Deleted Assets ========== | ||
| foreach (var assetPath in deletedAssets) | ||
| { | ||
| if (string.IsNullOrEmpty(assetPath)) continue; | ||
|
|
||
| // L0 Deduplication: Skip if already processed in this session | ||
| if (!_processedAssetsInSession.Add(assetPath)) | ||
| continue; | ||
|
|
||
| hasChanges = true; // Mark that we added a new entry | ||
|
|
||
| // L1 Blacklist: Skip junk assets | ||
| if (!EventFilter.ShouldTrackAsset(assetPath)) | ||
| continue; | ||
|
|
||
| string targetId = $"Asset:{assetPath}"; | ||
|
|
||
| var payload = new Dictionary<string, object> | ||
| { | ||
| ["path"] = assetPath | ||
| }; | ||
|
|
||
| RecordEvent(EventTypes.AssetDeleted, targetId, payload); | ||
| } | ||
|
|
||
| // ========== Moved Assets ========== | ||
| for (int i = 0; i < movedAssets.Length; i++) | ||
| { | ||
| if (string.IsNullOrEmpty(movedAssets[i])) continue; | ||
|
|
||
| // L0 Deduplication: Skip if already processed in this session | ||
| if (!_processedAssetsInSession.Add(movedAssets[i])) | ||
| continue; | ||
|
|
||
| hasChanges = true; // Mark that we added a new entry | ||
|
|
||
| var fromPath = i < movedFromAssetPaths.Length ? movedFromAssetPaths[i] : ""; | ||
|
|
||
| // L1 Blacklist: Skip junk assets | ||
| if (!EventFilter.ShouldTrackAsset(movedAssets[i])) | ||
| continue; | ||
|
|
||
| string targetId = $"Asset:{movedAssets[i]}"; | ||
|
|
||
| var payload = new Dictionary<string, object> | ||
| { | ||
| ["to_path"] = movedAssets[i], | ||
| ["from_path"] = fromPath | ||
| }; | ||
|
|
||
| RecordEvent(EventTypes.AssetMoved, targetId, payload); | ||
| } | ||
|
|
||
| // Persist the cache to disk if there were any changes | ||
| if (hasChanges) | ||
| SaveProcessedAssets(); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dedup cache suppresses legitimate future asset events.
Because _processedAssetsInSession never expires by time, any later import/modify/move on the same path in the same session is skipped until the cache clears (currently only when size > 1000). That risks losing real asset changes and contradicts the “time-based expiration” comment.
Consider tracking last-processed timestamps and expiring entries by age (e.g., 5–30 minutes), or scoping dedup to a batch window instead of the entire session.
[suggested approach]
- Replace the
HashSet<string>withDictionary<string, long>(last processed Unix ms). - Prune by TTL in
CleanupOldEntries. - Persist timestamps to disk so domain reloads keep the TTL semantics.
🛠️ Sketch of the intent (conceptual)
- private static HashSet<string> _cachedProcessedAssets;
+ private static Dictionary<string, long> _cachedProcessedAssets;
- if (!_processedAssetsInSession.Add(assetPath)) continue;
+ if (IsRecentlyProcessed(assetPath)) continue;
+ MarkProcessed(assetPath);
+ private static bool IsRecentlyProcessed(string path)
+ {
+ return _cachedProcessedAssets.TryGetValue(path, out var ts)
+ && (DateTimeOffset.UtcNow.ToUnixTimeMilliseconds() - ts) < ExpireWindowMs;
+ }Also applies to: 253-268
🤖 Prompt for AI Agents
In `@MCPForUnity/Editor/ActionTrace/Capture/Capturers/AssetCapture.cs` around
lines 145 - 246, The current dedup set _processedAssetsInSession prevents
legitimate later events because entries never expire; replace the
HashSet<string> with Dictionary<string,long> storing last-processed Unix-ms,
update the add/check logic in the three places that currently call
_processedAssetsInSession.Add(...) to instead prune via
CleanupOldEntries(ttlMs), then treat an entry as "already processed" only if
(now - lastProcessed) < ttlMs and otherwise update the timestamp and allow
processing; persist/load the dictionary in SaveProcessedAssets and the
corresponding load routine so timestamps survive domain reloads and implement
CleanupOldEntries to remove aged entries (TTL configurable, e.g., 5–30 minutes).
| static SelectionCapture() | ||
| { | ||
| // Initialize with current selection | ||
| UpdateSelectionState(); | ||
|
|
||
| // Monitor selection changes via HookRegistry | ||
| HookRegistry.OnSelectionChanged += OnSelectionChanged; | ||
|
|
||
| // Monitor property modifications via HookRegistry | ||
| HookRegistry.OnPropertiesModified += OnPropertyModified; | ||
|
|
||
| McpLog.Debug("[SelectionPropertyTracker] Initialized"); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing event unsubscription on domain reload.
The static constructor subscribes to HookRegistry.OnSelectionChanged and HookRegistry.OnPropertiesModified, but there's no corresponding unsubscription before assembly reload. This can cause issues with stale references after domain reload.
Additionally, the log message on line 46 references [SelectionPropertyTracker] but the class is named SelectionCapture.
🔧 Proposed fix
static SelectionCapture()
{
// Initialize with current selection
UpdateSelectionState();
// Monitor selection changes via HookRegistry
HookRegistry.OnSelectionChanged += OnSelectionChanged;
// Monitor property modifications via HookRegistry
HookRegistry.OnPropertiesModified += OnPropertyModified;
- McpLog.Debug("[SelectionPropertyTracker] Initialized");
+ McpLog.Debug("[SelectionCapture] Initialized");
+
+ // Subscribe to cleanup on domain reload
+ AssemblyReloadEvents.beforeAssemblyReload += OnBeforeAssemblyReload;
+}
+
+private static void OnBeforeAssemblyReload()
+{
+ HookRegistry.OnSelectionChanged -= OnSelectionChanged;
+ HookRegistry.OnPropertiesModified -= OnPropertyModified;
+ AssemblyReloadEvents.beforeAssemblyReload -= OnBeforeAssemblyReload;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| static SelectionCapture() | |
| { | |
| // Initialize with current selection | |
| UpdateSelectionState(); | |
| // Monitor selection changes via HookRegistry | |
| HookRegistry.OnSelectionChanged += OnSelectionChanged; | |
| // Monitor property modifications via HookRegistry | |
| HookRegistry.OnPropertiesModified += OnPropertyModified; | |
| McpLog.Debug("[SelectionPropertyTracker] Initialized"); | |
| } | |
| static SelectionCapture() | |
| { | |
| // Initialize with current selection | |
| UpdateSelectionState(); | |
| // Monitor selection changes via HookRegistry | |
| HookRegistry.OnSelectionChanged += OnSelectionChanged; | |
| // Monitor property modifications via HookRegistry | |
| HookRegistry.OnPropertiesModified += OnPropertyModified; | |
| McpLog.Debug("[SelectionCapture] Initialized"); | |
| // Subscribe to cleanup on domain reload | |
| AssemblyReloadEvents.beforeAssemblyReload += OnBeforeAssemblyReload; | |
| } | |
| private static void OnBeforeAssemblyReload() | |
| { | |
| HookRegistry.OnSelectionChanged -= OnSelectionChanged; | |
| HookRegistry.OnPropertiesModified -= OnPropertyModified; | |
| AssemblyReloadEvents.beforeAssemblyReload -= OnBeforeAssemblyReload; | |
| } |
🤖 Prompt for AI Agents
In `@MCPForUnity/Editor/ActionTrace/Capture/Capturers/SelectionCapture.cs` around
lines 35 - 47, The static ctor in SelectionCapture registers
HookRegistry.OnSelectionChanged -> OnSelectionChanged and
HookRegistry.OnPropertiesModified -> OnPropertyModified but never unsubscribes
before domain/assembly reload; add a cleanup subscription to
UnityEditor.AssemblyReloadEvents.beforeAssemblyReload (or
AssemblyReloadEvents.beforeAssemblyReload) that calls a new private static
UnsubscribeHooks() which removes those two handlers, and update the McpLog.Debug
tag from "[SelectionPropertyTracker]" to "[SelectionCapture]" so the log
reflects the correct class name.
| var evt = new EditorEvent( | ||
| sequence: 0, | ||
| timestampUnixMs: DateTimeOffset.UtcNow.ToUnixTimeMilliseconds(), | ||
| type: EventTypes.SelectionPropertyModified, | ||
| targetId: targetGlobalId, | ||
| payload: payload | ||
| ); | ||
|
|
||
| EventStore.Record(evt); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing sampling middleware check before recording.
Unlike Recorder.RecordEvent() which applies SamplingMiddleware.ShouldRecord(evt) before recording, this method directly calls EventStore.Record(evt). This inconsistency could lead to event floods during rapid property modifications (e.g., dragging a slider).
🔧 Proposed fix
+using MCPForUnity.Editor.ActionTrace.Capture;
// ... at the recording site:
var evt = new EditorEvent(
sequence: 0,
timestampUnixMs: DateTimeOffset.UtcNow.ToUnixTimeMilliseconds(),
type: EventTypes.SelectionPropertyModified,
targetId: targetGlobalId,
payload: payload
);
+ // Apply sampling middleware to prevent event floods
+ if (!SamplingMiddleware.ShouldRecord(evt))
+ return;
+
EventStore.Record(evt);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| var evt = new EditorEvent( | |
| sequence: 0, | |
| timestampUnixMs: DateTimeOffset.UtcNow.ToUnixTimeMilliseconds(), | |
| type: EventTypes.SelectionPropertyModified, | |
| targetId: targetGlobalId, | |
| payload: payload | |
| ); | |
| EventStore.Record(evt); | |
| var evt = new EditorEvent( | |
| sequence: 0, | |
| timestampUnixMs: DateTimeOffset.UtcNow.ToUnixTimeMilliseconds(), | |
| type: EventTypes.SelectionPropertyModified, | |
| targetId: targetGlobalId, | |
| payload: payload | |
| ); | |
| // Apply sampling middleware to prevent event floods | |
| if (!SamplingMiddleware.ShouldRecord(evt)) | |
| return; | |
| EventStore.Record(evt); |
🤖 Prompt for AI Agents
In `@MCPForUnity/Editor/ActionTrace/Capture/Capturers/SelectionCapture.cs` around
lines 162 - 170, The code creates an EditorEvent (evt) and calls
EventStore.Record(evt) directly; change this to apply the sampling middleware
check used by Recorder.RecordEvent(): call SamplingMiddleware.ShouldRecord(evt)
(or delegate to Recorder.RecordEvent(evt) if available) and only invoke
EventStore.Record(evt) when it returns true so rapid property changes are
throttled; ensure you reference the existing EditorEvent instance (evt) and
perform the check immediately before recording.
| HookRegistry.OnComponentAdded += OnComponentAdded; | ||
| HookRegistry.OnComponentRemoved += OnComponentRemoved; | ||
| HookRegistry.OnComponentRemovedDetailed += OnComponentRemovedDetailed; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential duplicate ComponentRemoved events.
Both OnComponentRemoved and OnComponentRemovedDetailed handlers are subscribed and both record events with type "ComponentRemoved". If HookRegistry fires both events for the same component removal, this will result in duplicate entries in the EventStore.
Note the comment on lines 49-50 explains why OnGameObjectDestroyed is skipped in favor of OnGameObjectDestroyedDetailed. Consider applying similar logic here.
🔧 Suggested approach
// Subscribe to HookRegistry events
HookRegistry.OnComponentAdded += OnComponentAdded;
- HookRegistry.OnComponentRemoved += OnComponentRemoved;
+ // Note: We only use OnComponentRemovedDetailed since it has complete cached data
+ // OnComponentRemoved may be called with partially valid data, so we skip it to avoid duplicates
HookRegistry.OnComponentRemovedDetailed += OnComponentRemovedDetailed;Also applies to: 108-140
🤖 Prompt for AI Agents
In `@MCPForUnity/Editor/ActionTrace/Capture/Recorder.cs` around lines 45 - 47,
Both OnComponentRemoved and OnComponentRemovedDetailed are subscribed and both
emit "ComponentRemoved" events, causing duplicates; mirror the
GameObjectDestroyed/Detailed fix by unsubscribing the non-detailed handler and
only handling the detailed event (or add a de-dup guard inside the handlers).
Concretely: remove or stop subscribing to HookRegistry.OnComponentRemoved in
Recorder.cs and ensure HookRegistry.OnComponentRemovedDetailed is the sole
source for recording component removals (also update the corresponding handler
logic that writes to EventStore/RecordEvent and remove any duplicate recording
logic in the alternative handler paths referenced around lines 108-140).
| .detail-scroll-view { | ||
| flex: 1; | ||
| min-width: 320px; | ||
| min-height: 100px; | ||
| max-width: 300px; | ||
| background-color: #252525; | ||
| border-left-width: 1px; | ||
| border-left-color: #3a3a3a; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Contradictory width constraints: min-width: 320px and max-width: 300px.
The detail panel has min-width: 320px but max-width: 300px, which is impossible to satisfy. This will cause unpredictable layout behavior. The max-width should likely be larger (e.g., 400px or 450px).
🔧 Proposed fix
.detail-scroll-view {
flex: 1;
min-width: 320px;
min-height: 100px;
- max-width: 300px;
+ max-width: 450px;
background-color: `#252525`;
border-left-width: 1px;
border-left-color: `#3a3a3a`;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .detail-scroll-view { | |
| flex: 1; | |
| min-width: 320px; | |
| min-height: 100px; | |
| max-width: 300px; | |
| background-color: #252525; | |
| border-left-width: 1px; | |
| border-left-color: #3a3a3a; | |
| } | |
| .detail-scroll-view { | |
| flex: 1; | |
| min-width: 320px; | |
| min-height: 100px; | |
| max-width: 450px; | |
| background-color: `#252525`; | |
| border-left-width: 1px; | |
| border-left-color: `#3a3a3a`; | |
| } |
🤖 Prompt for AI Agents
In
`@MCPForUnity/Editor/Windows/ActionTraceEditorWindow/ActionTraceEditorWindow.uss`
around lines 446 - 454, The .detail-scroll-view CSS rule has contradictory width
constraints (min-width: 320px vs max-width: 300px); update the rule so max-width
is >= min-width (for example set max-width to 400px or 450px) to remove the
conflict and ensure predictable layout behavior in ActionTraceEditorWindow.
| public void SetSortMode(SortMode sortMode) | ||
| { | ||
| if (ActionTraceSettings.Instance != null) | ||
| ActionTraceSettings.Instance.Filtering.BypassImportanceFilter = true; | ||
| _forceRefreshNextCall = true; | ||
| RefreshData(); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SetSortMode parameter is unused; sorting is hardcoded.
SetSortMode(SortMode sortMode) accepts a parameter but never stores or uses it. The comment at line 410 mentions "_sortMode" which doesn't exist, and ApplySorting always sorts by timestamp.
This appears to be incomplete implementation. Either implement the sort mode logic or remove the parameter:
♻️ Option: Store and use sort mode
+ private SortMode _sortMode = SortMode.TimeDescending;
+
public void SetSortMode(SortMode sortMode)
{
+ _sortMode = sortMode;
if (ActionTraceSettings.Instance != null)
ActionTraceSettings.Instance.Filtering.BypassImportanceFilter = true;
_forceRefreshNextCall = true;
RefreshData();
}
private IEnumerable<ActionTraceQuery.ActionTraceViewItem> ApplySorting(...)
{
- // Sort mode is controlled externally via _sortMode
- // For now, default to time-based sorting
- return source.OrderByDescending(e => e.Event.TimestampUnixMs);
+ return _sortMode switch
+ {
+ SortMode.TimeAscending => source.OrderBy(e => e.Event.TimestampUnixMs),
+ SortMode.ImportanceDescending => source.OrderByDescending(e => e.ImportanceScore),
+ _ => source.OrderByDescending(e => e.Event.TimestampUnixMs)
+ };
}Also applies to: 408-413
🤖 Prompt for AI Agents
In
`@MCPForUnity/Editor/Windows/ActionTraceEditorWindow/Components/EventListSection.cs`
around lines 290 - 296, SetSortMode currently ignores its parameter; add a
private field (e.g. _sortMode) to store the passed SortMode in the
EventListSection class, set _sortMode = sortMode inside SetSortMode, and update
ApplySorting to switch on _sortMode instead of always sorting by timestamp
(handle the different SortMode cases such as Timestamp, Importance, etc.). Also
update any comment references that mention _sortMode to match the new field and
ensure callers rely on SetSortMode to change sorting behavior; alternatively
remove the parameter and callers if you prefer to eliminate sort-by-mode
support.
| @mcp_for_unity_tool( | ||
| description="""⚠️ CALL BEFORE CHANGES - Prevents conflicts & wasted work | ||
|
|
||
| 15min summary of recent Unity ops. Detects conflicts before you break things. | ||
| Defaults: 15min window, medium importance. | ||
|
|
||
| Use when: modifying existing objects, debugging, or user mentions "刚才/之前/那个".""", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Align the tool description with the actual defaults.
The decorator says “15min summary” and “Defaults: 15min window,” but the function default is time_range="1h". Please update one or the other to avoid confusing callers.
Also applies to: 40-41
🤖 Prompt for AI Agents
In `@Server/src/services/tools/get_action_trace_summary.py` around lines 27 - 33,
The decorator description in `@mcp_for_unity_tool` claims a "15min summary" and
"Defaults: 15min window" but the function get_action_trace_summary has a default
time_range="1h" (also referenced on the description lines ~40-41); make them
consistent by either changing the function parameter default time_range to "15m"
(or an equivalent value used elsewhere) or updating the decorator description to
state the actual default "1h" in both places; update the description text and
any other decorator lines that mention the default so callers and docs match the
function signature.
| # Coerce and add optional parameters | ||
| if limit is not None: | ||
| coerced_limit = coerce_int(limit) | ||
| if coerced_limit is not None: | ||
| params_dict["limit"] = coerced_limit | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Enforce the documented 1–500 limit before sending to Unity.
limit is coerced but never range-validated, so 0/negative or huge values can slip through and cause backend errors or heavy payloads. Please enforce the contract here (clamp or reject).
🔧 Suggested fix
if limit is not None:
coerced_limit = coerce_int(limit)
if coerced_limit is not None:
- params_dict["limit"] = coerced_limit
+ if 1 <= coerced_limit <= 500:
+ params_dict["limit"] = coerced_limit
+ else:
+ return {
+ "success": False,
+ "message": "limit must be between 1 and 500"
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Coerce and add optional parameters | |
| if limit is not None: | |
| coerced_limit = coerce_int(limit) | |
| if coerced_limit is not None: | |
| params_dict["limit"] = coerced_limit | |
| # Coerce and add optional parameters | |
| if limit is not None: | |
| coerced_limit = coerce_int(limit) | |
| if coerced_limit is not None: | |
| if 1 <= coerced_limit <= 500: | |
| params_dict["limit"] = coerced_limit | |
| else: | |
| return { | |
| "success": False, | |
| "message": "limit must be between 1 and 500" | |
| } |
🤖 Prompt for AI Agents
In `@Server/src/services/tools/get_action_trace_summary.py` around lines 97 - 102,
The code currently coerces limit via coerce_int but doesn't enforce the
documented 1–500 range, allowing invalid values; update the block in
get_action_trace_summary (the section that handles limit/coerced_limit and
params_dict) to validate and enforce the range—either clamp coerced_limit to the
inclusive range 1..500 or raise/reject when out of range—then only set
params_dict["limit"] to the validated value (e.g., validated_limit = max(1,
min(coerced_limit, 500)) before assignment) so Unity always receives a limit
within 1–500.
| if limit is not None: | ||
| coerced_limit = coerce_int(limit) | ||
| if coerced_limit is not None: | ||
| params_dict["limit"] = coerced_limit | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Enforce the documented limit range (1–1000).
The annotation promises a bounded range, but any integer is currently accepted; negative or huge values could cause heavy payloads or Unity-side errors. Consider clamping or rejecting out-of-range values.
✅ Proposed guard
if limit is not None:
coerced_limit = coerce_int(limit)
if coerced_limit is not None:
+ if coerced_limit < 1 or coerced_limit > 1000:
+ return {
+ "success": False,
+ "message": "limit must be between 1 and 1000"
+ }
params_dict["limit"] = coerced_limit📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if limit is not None: | |
| coerced_limit = coerce_int(limit) | |
| if coerced_limit is not None: | |
| params_dict["limit"] = coerced_limit | |
| if limit is not None: | |
| coerced_limit = coerce_int(limit) | |
| if coerced_limit is not None: | |
| if coerced_limit < 1 or coerced_limit > 1000: | |
| return { | |
| "success": False, | |
| "message": "limit must be between 1 and 1000" | |
| } | |
| params_dict["limit"] = coerced_limit |
🤖 Prompt for AI Agents
In `@Server/src/services/tools/get_action_trace.py` around lines 64 - 68, The
current handling of limit in get_action_trace (variables: limit, coerced_limit,
coerce_int, params_dict["limit"]) accepts any integer; modify the logic to
enforce the documented 1–1000 range by validating coerced_limit after coerce_int
returns a value and either clamp it into the [1,1000] range or raise a clear
validation error; then only assign the bounded value to params_dict["limit"].
Ensure you perform this check where coerced_limit is set so out-of-range
negatives or huge values are prevented before being sent downstream.
Hook System OverviewPurposeHookSystem is an event bus layer that allows systems to subscribe to Unity editor events without directly depending on Unity APIs. ActionTrace subscribes to all editor events through HookSystem, recording user operation history so AI can understand "what just happened." Core Benefits:
Available Events
Files That Could Use HookSystem✅ Already Migrated
🔶 Could Migrate (Needs Evaluation)
❌ Not Recommended
Usage Example// Subscribe to events
HookRegistry.OnGameObjectCreated += (go) => Debug.Log($"Created: {go.name}");
HookRegistry.OnScriptCompiled += () => RefreshMySystem();
// Unsubscribe
HookRegistry.OnGameObjectCreated -= myHandler; |
ActionTrace: What it can do / What it cannot do✅ What it CAN do
❌ What it CANNOT do
In one sentence: ActionTrace is AI's "memory notebook" — it remembers what you did, but doesn't act on its own. |
…ce.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…cs.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…gs.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@MCPForUnity/Editor/ActionTrace/Core/Settings/ActionTraceSettings.cs`:
- Around line 195-205: CreateSettings calls
ApplyPreset(ActionTracePreset.Standard) before the asset exists on disk, but
ApplyPreset calls Save() which uses EditorUtility.SetDirty(this) and
AssetDatabase.SaveAssets() causing warnings; to fix, delay applying the preset
until after the asset is created on disk or inline the preset values directly:
move the ApplyPreset(ActionTracePreset.Standard) call to after
AssetDatabase.CreateAsset(settings, SettingsPath) (and keep subsequent
AssetDatabase.SaveAssets()/Refresh()), or alternatively set the fields on the
new settings instance instead of calling ApplyPreset/Save before creation;
reference CreateSettings, ApplyPreset, Save, AssetDatabase.CreateAsset,
SettingsPath when updating the code.
♻️ Duplicate comments (1)
MCPForUnity/Editor/ActionTrace/Core/Settings/ActionTraceSettings.cs (1)
254-260:GetEstimatedMemoryUsagecan return negative if settings are invalid.Line 258 computes
coldEvents = Storage.MaxEvents - Storage.HotEventCount. IfHotEventCount > MaxEvents(whichValidate()warns about but doesn't prevent),coldEventsbecomes negative, yielding an incorrect memory estimate.♻️ Suggested defensive fix
public long GetEstimatedMemoryUsage() { // Approximate: each event ~300 bytes when hydrated, ~100 bytes when dehydrated int hotEvents = Storage.HotEventCount; - int coldEvents = Storage.MaxEvents - Storage.HotEventCount; + int coldEvents = Math.Max(0, Storage.MaxEvents - Storage.HotEventCount); return (long)(hotEvents * 300 + coldEvents * 100); }
🧹 Nitpick comments (1)
MCPForUnity/Editor/ActionTrace/Core/Store/EventStore.Persistence.cs (1)
136-145:_saveScheduledreset outside lock could race withScheduleSave.Line 143 sets
_saveScheduled = falseoutside any lock, whileScheduleSavechecks and sets this flag under_queryLock. In a multi-threaded scenario this could cause a race. However, since this is called during shutdown/domain reload when other operations shouldn't be active, and Unity Editor is predominantly single-threaded, this is low-risk.♻️ Optional: wrap in lock for consistency
public static void ClearPendingOperations() { lock (_pendingNotifications) { _pendingNotifications.Clear(); _notifyScheduled = false; } - _saveScheduled = false; + lock (_queryLock) + { + _saveScheduled = false; + } _lastDehydratedCount = -1; // Reset dehydration optimization marker }
| private static ActionTraceSettings CreateSettings() | ||
| { | ||
| var settings = CreateInstance<ActionTraceSettings>(); | ||
| // Apply Standard preset by default | ||
| settings.ApplyPreset(ActionTracePreset.Standard); | ||
| AssetDatabase.CreateAsset(settings, SettingsPath); | ||
| AssetDatabase.SaveAssets(); | ||
| AssetDatabase.Refresh(); | ||
| McpLog.Info($"[ActionTraceSettings] Created new settings at {SettingsPath}"); | ||
| return settings; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ApplyPreset calls Save() before the asset exists on disk.
ApplyPreset(ActionTracePreset.Standard) internally calls Save(), which invokes EditorUtility.SetDirty(this) and AssetDatabase.SaveAssets(). However, at this point, the asset hasn't been created yet (that happens on line 200). This likely generates a warning or fails silently.
🔧 Proposed fix: defer preset application or inline the values
private static ActionTraceSettings CreateSettings()
{
var settings = CreateInstance<ActionTraceSettings>();
- // Apply Standard preset by default
- settings.ApplyPreset(ActionTracePreset.Standard);
+ // Apply Standard preset values directly (without Save)
+ var preset = ActionTracePreset.Standard;
+ settings.Filtering.MinImportanceForRecording = preset.MinImportance;
+ settings.Storage.MaxEvents = preset.MaxEvents;
+ settings.Storage.HotEventCount = preset.HotEventCount;
+ settings.Merging.EnableEventMerging = preset.EnableEventMerging;
+ settings.Merging.MergeWindowMs = preset.MergeWindowMs;
+ settings.Merging.TransactionWindowMs = preset.TransactionWindowMs;
+ settings._currentPresetName = preset.Name;
+
AssetDatabase.CreateAsset(settings, SettingsPath);
AssetDatabase.SaveAssets();
AssetDatabase.Refresh();
McpLog.Info($"[ActionTraceSettings] Created new settings at {SettingsPath}");
return settings;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private static ActionTraceSettings CreateSettings() | |
| { | |
| var settings = CreateInstance<ActionTraceSettings>(); | |
| // Apply Standard preset by default | |
| settings.ApplyPreset(ActionTracePreset.Standard); | |
| AssetDatabase.CreateAsset(settings, SettingsPath); | |
| AssetDatabase.SaveAssets(); | |
| AssetDatabase.Refresh(); | |
| McpLog.Info($"[ActionTraceSettings] Created new settings at {SettingsPath}"); | |
| return settings; | |
| } | |
| private static ActionTraceSettings CreateSettings() | |
| { | |
| var settings = CreateInstance<ActionTraceSettings>(); | |
| AssetDatabase.CreateAsset(settings, SettingsPath); | |
| // Apply Standard preset by default (after asset is registered) | |
| settings.ApplyPreset(ActionTracePreset.Standard); | |
| AssetDatabase.SaveAssets(); | |
| AssetDatabase.Refresh(); | |
| McpLog.Info($"[ActionTraceSettings] Created new settings at {SettingsPath}"); | |
| return settings; | |
| } |
🤖 Prompt for AI Agents
In `@MCPForUnity/Editor/ActionTrace/Core/Settings/ActionTraceSettings.cs` around
lines 195 - 205, CreateSettings calls ApplyPreset(ActionTracePreset.Standard)
before the asset exists on disk, but ApplyPreset calls Save() which uses
EditorUtility.SetDirty(this) and AssetDatabase.SaveAssets() causing warnings; to
fix, delay applying the preset until after the asset is created on disk or
inline the preset values directly: move the
ApplyPreset(ActionTracePreset.Standard) call to after
AssetDatabase.CreateAsset(settings, SettingsPath) (and keep subsequent
AssetDatabase.SaveAssets()/Refresh()), or alternatively set the fields on the
new settings instance instead of calling ApplyPreset/Save before creation;
reference CreateSettings, ApplyPreset, Save, AssetDatabase.CreateAsset,
SettingsPath when updating the code.
|
@whatevertogo Thanks again for your work here. We still think it's not a fit for the mcp repo right now for reasons mentioned before. So I'll close the PR tomorrow. But by all means keep developing the feature independently, and as we said, we're happy to track your progress and have you share it on the Discord etc. Thanks again! |
|
Understood |
ActionTrace System - Unity Editor Event Tracking
Type of Change
Changes Made
Overview
ActionTrace is a comprehensive Unity Editor event tracking and recording system that provides complete event context for AI assistants. This PR introduces the complete ActionTrace architecture with 8000 lines of code across 60+ files.
Core Architecture
Foundation Layer (
Core/):EditorEvent.cs- Event data model with serialization supportEventStore.cs- Central event storage with hot/cold event managementEventStore.Persistence.cs- Domain reload survival and disk persistenceEventStore.Diagnostics.cs- Memory management and event dehydrationEventStore.Merging.cs- Event deduplication and transaction aggregationEventStore.Context.cs- Context mapping managementActionTraceSettings.cs- Layered settings architecture (Filtering, Merging, Storage)ActionTracePreset.cs- Preset system (Standard, Verbose, Minimal, Silent)EventTypes.cs,EventCategory.cs,EventMetadata.cs- Type definitionsCapture Layer (
Capture/):Recorder.cs- Central recording orchestratorEmitter.cs- Event emission coordinationCapturers/AssetCapture.cs- Asset import/modify/delete trackingCapturers/PropertyCapture.cs- Property modification trackingCapturers/SelectionCapture.cs- Selection change trackingFilters/EventFilter.cs- Rule-based event filtering (Prefix, Extension, Regex, GameObject)Sampling/SamplingMiddleware.cs- High-frequency event throttlingSampling/SamplingStrategy.cs- Pluggable sampling algorithmsSampling/SamplingConfig.cs- Sampling configurationAnalysis Layer (
Analysis/):Query/ActionTraceQuery.cs- Fluent query API with filtering and aggregationSummarization/EventSummarizer.cs- AI-friendly event summariesSummarization/TransactionAggregator.cs- Transaction groupingContext & Semantics:
Context/ContextMapping.cs- Operation context trackingSemantics/DefaultEventScorer.cs- Event importance scoring (0-10)Semantics/DefaultIntentInferrer.cs- Intent inference from eventsSemantics/DefaultCategorizer.cs- Event categorizationIntegration:
Integration/AssetBridge.cs- Asset operation trackingIntegration/VCS/VcsContextProvider.cs- Version control contextHelpers:
Helpers/GlobalIdHelper.cs- Cross-session stable object identificationHelpers/GameObjectTrackingHelper.cs- GameObject lifecycle trackingHelpers/PropertyEventPayloadBuilder.cs- Consistent payload structureHelpers/PropertyFormatter.cs- Property value formattingHelpers/PropertyModificationHelper.cs- Modification detectionHelpers/BuildTargetUtility.cs- Build target detectionIndependent Hook System (
Hooks/):Hooks/HookRegistry.cs- Dependency-injected hook registration system (decoupled from ActionTrace)Hooks/UnityEventHooks/- Comprehensive Unity event hooksHooks/EventArgs/- Type-safe event argument definitionsUI (
Windows/):ActionTraceEditorWindow.cs- Editor window with timeline, filtering, and exportActionTraceEditorWindow.uxml- UI layoutActionTraceEditorWindow.uss- UI stylingMCP Tools:
GetActionTraceTool.cs- Query events with filteringGetActionTraceSummaryTool.cs- AI-friendly aggregated summariesAddActionTraceNoteTool.cs- Add timeline notesActionTraceSettingsTool.cs- Settings managementPython Server:
Server/src/models/action_trace.py- Event data modelsServer/src/services/resources/action_trace.py- Event stream resourceServer/src/services/tools/get_action_trace.py- Query tool wrapperServer/src/services/tools/get_action_trace_summary.py- Summary tool wrapperServer/src/services/tools/add_action_trace_note.py- Note tool wrapperKey Features
Hot/Cold Event Storage
Cross-Session Stability
Asset:{path}formatEvent Filtering & Sampling
Query & Analysis
Context Awareness
Preset System
Performance Optimizations
Breaking Changes
None - All changes are backward compatible.
Testing/Screenshots/Recordings
Manual Testing
Test Scripts
Hooks/HookTest.cs- Hook validation scriptDocumentation Updates
Documentation Updates Using
tools/UPDATE_DOCS_PROMPT.md(recommended)tools/UPDATE_DOCS.mdUpdated Files
README.md- Added ActionTrace tools to available tools listmanifest.json- Registered new ActionTrace toolsdocs/i18n/README-zh.md- Chinese documentation updatesCLAUDE.md- Added ActionTrace architecture sectionAdditional Notes
Architecture Evolution
This PR represents the culmination of 16 major commits:
Future Considerations
Summary by Sourcery
Implement comprehensive Unity Editor event tracking system for AI-assisted development context.
Features:
Event Capture & Storage:
Query & Analysis:
Architecture:
Integration:
Performance:
Technical Details:
Summary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.