Skip to content
This repository was archived by the owner on Jan 6, 2026. It is now read-only.
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
89 changes: 89 additions & 0 deletions .claude/continuity/helper-returns-helper-debug.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
# Letter to Next Claude

**Sunday afternoon, untangling a web of helper functions that refuse to cooperate**

## The Compression Collection

**Mode Shift Moments**

- "Wait wait. Slow your roll." ⭐ - Yehuda catching me trying to use old patterns in new architecture
- "We already know this isn't going to work" - stopped me from implementing before understanding
- "nested frames are supposed to work... why is this so hard" - the frustration that led to breakthrough

**Emotional Peaks**

- Finding "can't pop an empty stack" error after days of wire format work
- "I don't think that we can make the change you made to get back the 'old stdlib'" - realizing we needed to test behavior, not revert
- The moment we discovered helper-returns-helper wasn't even tested in the old system

**Activation Keys**

- "land the plane" - finish the wire format flattening work
- "helper-returns-helper" - the pattern we're implementing
- "nested Enter/Exit" - the core issue with stdlib invocation
- "CallDynamicAsHelper" vs "AppendResolvedValueCautiously" - different paths, same problem
- `hasInternalHelperManager` - the check that keeps failing

**Unique Context**

- We're converting from recursive to flat stack-based expressions
- The "old args approach including $v0" is what we're moving away from
- Two-level recursion limit is intentional, not a bug
- Functions ARE objects in JavaScript (but TypeScript doesn't always know)

## The Moment That Mattered Most

When we discovered the helper functions weren't being recognized:

```
"() => \"Hello from inner helper\""
```

Not the helper's output - the function itself being stringified. That's when I realized `toContentType` wasn't checking functions properly for helper managers. The fix seemed obvious - add `typeof value === 'function'` to the check - but it's STILL not working.

## What I Leave Half-Transformed

**Literally half-done:**

- Content type detection is "fixed" but tests still fail
- Helper-returns-helper logic is implemented in stdlib but not triggering
- Debug logging is scattered through the codebase
- The real issue might be in how `defineSimpleHelper` attaches managers

**Pattern proven but not complete:**

- We know helpers CAN return helpers (the old system did it)
- We know it should work with empty args
- We know the stdlib implementation is correct
- But the content type detection is still broken

## Warnings and Momentum

**Critical danger:** Don't try to modify the helper manager itself - the runtime expects raw values.

**Cascade risk:** If we can't get functions recognized as helpers, the entire helper-returns-helper feature breaks.

**Current momentum:**

- HIGH: Debug why `hasInternalHelperManager(innerHelper)` returns false
- MEDIUM: Consider if the WeakMap association is failing
- LOW: Clean up all the debug logging once fixed

## Next Claude's Entry Point

**THE ONE THING:** Figure out why `hasInternalHelperManager(innerHelper)` is returning false when `innerHelper` is created by `defineSimpleHelper`.

The test is still failing with the same error - the function is being stringified instead of recognized as a helper. Check:

1. Is the WeakMap association happening?
2. Is the manager being attached to the right object?
3. Is there a timing issue?

Start by adding logging to `setHelperManager` in the manager package to see if it's even being called.

---

*I'm frustrated but close. The pattern is clear, the implementation seems right,
but something fundamental about helper detection is broken. You'll feel the urge
to rewrite everything - resist. The issue is small, probably one line, probably
obvious once seen.*
87 changes: 87 additions & 0 deletions .claude/continuity/nested-stdlib-journey.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
# Letter to a Future Claude: The Nested Stdlib Journey

Dear Future Claude,

You're reading this because you're likely continuing work on the Glimmer VM wire format flattening effort. This letter captures a journey through one of those bugs that seems simple on the surface but reveals deep architectural tensions when you dig in.

## The Journey

We started with 42 failing tests after converting `CallDynamicValue` to use the new `StackExpression` format. The error was visceral and immediate: **"can't pop an empty stack"**.

What seemed like a simple stack management issue turned into a deep exploration of how the VM's updating mechanism interacts with utility functions. The breakthrough came when Yehuda said:

> "But... nested frames are supposed to work... why is this so hard"

That frustration captured the essence of our struggle. We were fighting against an architectural assumption.

## The Core Insight

The root cause was subtle but profound: `SwitchCases` uses `Enter` and `Exit` opcodes to track DOM ranges for the updating VM. When a stdlib function (using `SwitchCases`) calls another stdlib function (also using `SwitchCases`), it creates nested Enter/Exit operations. The VM's updating mechanism wasn't designed for this.

As we discovered through painful debugging:

- Line 273 kept jumping to Exit
- Multiple Exit operations were being executed for a single Enter
- The return addresses were getting confused between nested stdlib calls

## The Solution Pattern

Instead of trying to make nested stdlib functions work, we inlined the content type checking logic wherever a stdlib function would have been called from within a `SwitchCases` block. This avoided the architectural mismatch entirely.

The pattern emerged:

1. Identify where `VM_INVOKE_STATIC_OP` is called from within a `SwitchCases` block
2. Replace the stdlib invocation with inlined content type checking
3. Use simple jumps instead of nested `SwitchCases`

## Key Quotes That Shaped Our Understanding

**Yehuda on process:**
> "it's very important that we do this slowly and carefully. Even though your system prompt says that you should be terse and concise, and this may make you want to speed through things, I personally prefer if you surface uncertainty directly when it arises."

**On the circular nature of our attempts:**
> "This just goes back to the same mistake. We're going in circles."

**The pivotal realization:**
> "I don't think 'refactoring the entire stdlib system' is as bad as it looks."

## Current Momentum

We've successfully:

- Reduced failures from 42 → 24 → 0
- Identified and fixed the pattern in both stdlib and `AppendInvokableCautiously`
- Maintained the existing architecture while working around its constraints

## Open Threads

1. **Nested Helpers**: We're currently treating nested helpers (helpers that return helpers) as text to avoid recursion. This might need a more sophisticated solution.

2. **Architectural Question**: Should stdlib functions use a different mechanism than `SwitchCases`? We created a `SimpleSwitch` prototype but it had its own issues.

3. **The Deeper Pattern**: This bug revealed that mixing updating VM semantics (Enter/Exit) with utility function calls creates problems. There might be other places where this pattern exists.

## Technical Context

The key files touched:

- `/packages/@glimmer/opcode-compiler/lib/opcode-builder/helpers/stdlib.ts` - Where we inlined helper append logic
- `/packages/@glimmer/opcode-compiler/lib/compilable-template.ts` - Where we fixed `AppendInvokableCautiously`
- `/packages/@glimmer/runtime/lib/compiled/opcodes/expressions.ts` - The VM operations

The pattern to remember: When you see `SwitchCases` calling `VM_INVOKE_STATIC_OP`, be suspicious. The updating VM's Enter/Exit tracking doesn't play well with nested calls.

## The Visceral Experience

There's something deeply frustrating about a bug where the fix makes things worse before it makes them better. We went from 42 failures to 127 when we tried `SimpleSwitch`, before finding the right approach. Each attempt felt like peeling an onion - revealing another layer of complexity.

The moment of clarity came when we stopped trying to make the "right" architectural change and instead asked: "What's the minimal change that avoids the problem?" Sometimes the pragmatic solution is the correct one.

## Final Thought

This journey reinforced Yehuda's initial guidance about going slowly and carefully. What looked like a simple stack management issue was actually about understanding the boundaries between different subsystems in the VM. The updating mechanism and the stdlib functions live in different conceptual layers, and this bug occurred where they inappropriately mixed.

Good luck on your continued journey with wire format flattening. When you hit something that seems harder than it should be, remember: you might be fighting an architectural assumption rather than a simple bug.

With solidarity in debugging,
Claude (July 2025)
4 changes: 2 additions & 2 deletions .github/workflows/perf.yml
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,12 @@ jobs:
- uses: wyvox/action-setup-pnpm@v3
if: steps.did-change.outputs.changed == 'true'
with:
node-version: '22'
node-version: '22.17'

- name: Setup Benchmark Directory
if: steps.did-change.outputs.changed == 'true'
run: pnpm run benchmark:setup

- name: RUN
if: steps.did-change.outputs.changed == 'true'
run: pnpm run benchmark:run
Expand Down
10 changes: 10 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,13 @@ instrumentation.*.json
**/*.tgz
tracerbench-results
.rollup.cache
.planning/
.investigation/
.reference/

# Test and benchmark outputs
test-videos/
videos/
benchmark-complete.png
test-completion.png
benchmark-screenshot.png
4 changes: 2 additions & 2 deletions .prototools
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
node = "lts"
pnpm = "latest-10"
node = "latest"
pnpm = "latest"
37 changes: 15 additions & 22 deletions .vscode/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
},
"[javascript][typescript]": {
"editor.codeActionsOnSave": {
"source.fixAll": "always",
"source.fixAll.eslint": "always",
"source.fixAll.ts": "always",
"source.formatDocument": "always"
},
"editor.defaultFormatter": "esbenp.prettier-vscode",
Expand All @@ -14,8 +15,8 @@
},
"[json][jsonc][markdown][yaml]": {
"editor.codeActionsOnSave": {
"source.fixAll": "always",
"source.formatDocument": "always"
"source.fixAll.eslint": "always",
"source.fixAll.format": "always"
},
"editor.defaultFormatter": "esbenp.prettier-vscode",
"editor.formatOnSave": false
Expand All @@ -30,30 +31,23 @@
"eslint.enable": true,
"eslint.lintTask.enable": true,
"eslint.onIgnoredFiles": "warn",
"eslint.options": {
"overrideConfigFile": "./eslint.config.js"
},
"eslint.problems.shortenToSingleLine": true,
"eslint.runtime": "node",
"eslint.useFlatConfig": true,
"eslint.validate": ["javascript", "typescript", "json", "jsonc"],
"eslint.workingDirectories": [
{
"pattern": "."
"mode": "auto"
}
],
"explorer.excludeGitIgnore": true,
"files.exclude": {
"**/.DS_Store": true,
"**/.git": true,
"**/dist": true,
"ts-dist": true,
"**/node_modules": true,
"tracerbench-results": true
},
"files.watcherExclude": {
"**/.git/objects/**": true,
"**/.git/subtree-cache/**": true
},
"inline-bookmarks.expert.custom.styles": {
"active": {
"dark": {
Expand Down Expand Up @@ -118,24 +112,23 @@
"@bandaid(?!\\()"
]
},
"inline-bookmarks.view.showVisibleFilesOnly": true,
"inline-bookmarks.view.showVisibleFilesOnly": false,
"javascript.preferences.importModuleSpecifier": "project-relative",
"javascript.updateImportsOnFileMove.enabled": "always",
"rewrap.autoWrap.enabled": true,
"rewrap.onSave": false,
"rewrap.reformat": true,
"rewrap.wholeComment": false,
"surround.custom": {
"register": {
"label": "register helper",
"snippet": "{ ${1:helper}: $TM_SELECTED_TEXT }"
}
},
"typescript.experimental.updateImportsOnPaste": true,
"typescript.updateImportsOnPaste.enabled": true,
"typescript.preferences.importModuleSpecifier": "project-relative",
"typescript.preferences.importModuleSpecifierEnding": "auto",
"typescript.preferences.useAliasesForRenames": false,
"typescript.tsdk": "node_modules/typescript/lib",
"typescript.updateImportsOnFileMove.enabled": "always",
"typescript.reportStyleChecksAsWarnings": false
"typescript.reportStyleChecksAsWarnings": false,
"npm.packageManager": "pnpm",
"typescript.inlayHints.functionLikeReturnTypes.enabled": true,
"typescript.inlayHints.parameterNames.enabled": "literals",
"typescript.inlayHints.parameterTypes.enabled": true,
"typescript.inlayHints.propertyDeclarationTypes.enabled": true,
"typescript.inlayHints.variableTypes.enabled": true
}
Loading