Skip to content

Fix TransformerBridge temporary hook cleanup - #1638

Merged
jlarson4 merged 2 commits into
TransformerLensOrg:dev-4.xfrom
emerardd:fix/bridge-hook-lifecycle
Aug 11, 2026
Merged

Fix TransformerBridge temporary hook cleanup#1638
jlarson4 merged 2 commits into
TransformerLensOrg:dev-4.xfrom
emerardd:fix/bridge-hook-lifecycle

Conversation

@emerardd

Copy link
Copy Markdown
Contributor

Description

Summary

  • Give TransformerBridge temporary hook helpers scoped context-level ownership.
  • Tag user, cache, gradient-cache, and stop hooks with the current helper level, then remove only that level during cleanup.
  • Preserve pre-existing, outer-context, backward, and permanent hooks across hooks(), run_with_hooks(), and run_with_cache().
  • Delay run_with_cache() hook attachment until input preprocessing is complete, and make partial hook installation exception-safe.
  • Add offline regression coverage for ordinary and nested contexts, exception cleanup, backward caching, aliases, permanent hooks, and reset_hooks_end=False.

Root cause and impact

BridgeCore previously tracked only each touched HookPoint and direction, then called unscoped remove_hooks(dir=...) when a temporary helper exited. That removed every non-permanent hook in the selected direction, including hooks installed before the helper call or owned by an outer context.

This could silently disable an intervention or recording hook after an otherwise successful cache/helper call. Later experiments would continue running without the intended hook and without an exception.

The fix mirrors the established HookedRootModule lifecycle contract: each helper invocation receives a context level, every temporary hook it owns is tagged with that level, and exception-safe cleanup removes only that level. Public reset_hooks_end=False behavior remains unchanged.

Fixes #1636

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Screenshots

N/A — code-only hook lifecycle fix.

Validation

  • HookPoint, hook-orchestration, and native-Bridge unit surface: 89 passed.
  • Affected cache/stop/backward cross-component surface: 9 passed, 17 deselected.
  • mypy .: no issues in 424 source files.
  • pycln, isort, Black, and git diff --check: passed.
  • The complete test suite was not rerun locally.

Checklist:

  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation (no documentation change required; this restores the existing temporary-hook contract)
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes (the affected unit-test surface passed; the complete unit suite was not rerun)
  • I have not rewritten tests relating to key interfaces which would affect backward compatibility

@emerardd

Copy link
Copy Markdown
Contributor Author

CI update: all code-related checks on head ede493b are green, including compatibility checks on Python 3.10/3.11/3.12, full coverage, format, type, docstring, and benchmark checks. The two remaining failed notebook jobs (Attribution_Patching_Demo and Activation_Patching_in_TL_Demo) failed while downloading unchanged Hugging Face models with HTTP 429 Too Many Requests (gpt2 and NeelNanda/Attn_Only_2L512W_C4_Code); the later NameErrors are cascading from those load failures. This PR does not modify either notebook. Marking this ready for review.

@emerardd
emerardd marked this pull request as ready for review August 11, 2026 04:17
@jlarson4

Copy link
Copy Markdown
Collaborator

Great work on this @emerardd! Approved and merging as is.

@jlarson4
jlarson4 merged commit 4f5042c into TransformerLensOrg:dev-4.x Aug 11, 2026
48 of 50 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug Report] TransformerBridge temporary hook helpers remove pre-existing non-permanent hooks

2 participants