[docs]: surface activation-trace utility in add-model skills - #1399
Merged
SolitaryThinker merged 1 commit intoMay 26, 2026
Conversation
PR hao-ai-lab#1304 (commit a75d197) merged the activation-trace docs + nav wiring, but the add-model* skills still describe a DIY register_forward_hook workflow that duplicates the new first-class utility at fastvideo/hooks/activation_trace.py. This PR makes the skills surface and use the new utility: - add-model (umbrella), add-model-02-parity, add-model-03-port-dit, add-model-09-pipeline: small routing inserts pointing future-agents at add-model-08-trace + docs/contributing/activation_trace.md when component or pipeline parity is numerically red. - add-model-08-trace: replace DIY forward-hook workflow with activation trace as the primary path; demote custom hooks/monkey-patches/source edits to a Fallback Instrumentation Hierarchy. Updated frontmatter, Trace Compare Contract, Drill-Down Loop, Handoff bullets, and References. Net: ~126 added / ~119 removed across 5 skill files, ~245 touched lines. Pure markdown; no production code or tests touched.
Contributor
Merge ProtectionsYour pull request matches the following merge protections and will not be merged until they are valid. 🔴 PR merge requirementsWaiting for
This rule is failing.
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request updates several agent skill markdown files to prioritize using FastVideo's built-in activation trace utility (fastvideo/hooks/activation_trace.py) for debugging numerical drift before falling back to custom hooks or monkey-patching. The reviewer suggested wrapping PATCH_LINEAR=1 in backticks in add-model-08-trace/SKILL.md for consistency.
| - Hypothesis verdict if an A/B toggle was used (e.g. "PATCH_LINEAR=1 closes | ||
| the gap, confirming dtype-cast difference in PackedExpertLinear"). | ||
| - Fallback hook/patch verdict if activation trace could not observe the boundary. | ||
| - Hypothesis verdict if an A/B toggle was used, for example `PATCH_LINEAR=1`. |
Contributor
There was a problem hiding this comment.
For consistency with other environment variables and code snippets in this document, PATCH_LINEAR=1 should be wrapped in backticks.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
PR #1304 (commit `a75d19786a`) merged the activation-trace docs and mkdocs nav wiring, exposing `fastvideo/hooks/activation_trace.py` as the canonical layer-by-layer divergence tool. The add-model* skills still describe a DIY `register_forward_hook` workflow that duplicates the new utility.
This PR makes the skills surface and use the new utility.
Files
Net: ~126 added / ~119 removed, ~245 touched lines across 5 files. Pure markdown.
Strategic decision
Replaced the DIY workflow in `add-model-08-trace` rather than adding activation-trace as a sibling section. Reason: keeping both inline as peers would preserve drift and teach future-agents to bypass the zero-residue JSONL path. The DIY hooks/patches survive as a "Fallback Instrumentation Hierarchy" subsection for cases where activation-trace cannot observe the needed boundary.
Verification
Test plan