fix(debug): stepInstruction nested-calls + 100% coverage#107
Merged
Conversation
CI on the v7→master integration PR caught two issues in alpha.7's
stepInstruction implementation:
1. Nested calls (foo calls bar from inside foo): the helper's
scope-name comparison surfaced at bar::2 instead of running through
bar's body to land at foo::3. Cause: arrivalPath.scope is the
immediate callee (not the full stack), so 'sibling scope at same
depth' (bar from foo's POV) was misclassified as 'different
instruction.'
2. PostDebugSession.ts coverage dropped to 97.5% (lines 251, 258
uncovered) — branches that exercise non-empty click.scope weren't
reached by the existing tests.
Fix: classify scope changes by length relative to click, using Post's
call stack discipline (returns always go through ancestor scopes):
- cur.length > click.length → deeper, keep stepping
- cur.length < click.length → shallower, surface (returned past click)
- equal length: scope-content match → sub-step / land by index;
scope-content differ → sibling callee at same depth, keep stepping
New tests cover both nested-call (foo→bar→foo::3) and mid-group
(10.2→10.3→20) cases, exercising the previously uncovered branches.
Existing 'inside callee at last' test simplified to use foo: {1: stop}
(single-instruction subroutine) so the test reflects the helper's
shallower-scope detection path.
…lpha.8 Engine alpha.8 (TapeSnapshot + tapeViewport move, #227) shipped on npm next today. Bump post's peer-dep widening for alpha.7 to track the latest engine alpha at ship time. Updates: package.json peer dep, package-lock.json, CHANGELOG entry, CLAUDE.md narrative + #stillIn- ClickTimeInstruction algorithm description.
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
CI on the v7→master integration PR (action run) caught two issues in alpha.7's
stepInstruction()(#101):foocallsbarfrom insidefoo): the helper's scope-name comparison surfaced atbar::2instead of running through bar's body to land atfoo::3. Cause:arrivalPath.scopeis the immediate callee (not the full stack), so "sibling scope at same depth" (barfrom foo's POV) was misclassified as "different instruction".PostDebugSession.tsat 97.5% (lines 251, 258 uncovered). The two branches needed non-emptyclick.scopeto exercise; existing tests all clicked at main-scope.Fix
Classify scope changes by length relative to click, using Post's call-stack discipline (returns always go through ancestor scopes):
cur.length > click.length→ deeper, keep stepping.cur.length < click.length→ shallower, surface (returned past click).This recovers the user's "next numbered in current scope" semantics for nested calls without needing engine-internal stack access.
Tests
nested call from non-main scope— covers the bar-from-foo case (foo::2callsbar; stepInstruction lands atfoo::3after bar completes).mid-group → next numbered— covers the same-scope, same-index sub-step transition (10.2 → 10.3 → 20). Uses20: right, 30: stopbecause the engine doesn't pause before terminal stops.inside callee at last numberedtest simplified to usefoo: {1: stop}— exercises the shallower-scope return path directly.Verification
npm run test:coverage— 100% across statements / branches / functions / lines (was 97.5% statements / 97.14% branches in PostDebugSession.ts)npm run check/lintcleanSequencing
Should land on
v7before alpha.7 is published — current alpha.7 source on v7 has the bug. After this merges, the publish frompackages/machine/ships the fixed version.