fix(spawn): bind every real positional arg of a spawned callable (calque#191) - #193
Merged
Merged
Conversation
… just the first (calque#191) SpawnCallable.MethodArg only ever carried the FIRST non-self/cls parameter name -- a callable like AI-Almanac's forecasts_app.py's run_forecast_inference(job_id, model_id, config) would (once calque#189's target-resolution fix landed) build a real shard but silently leave model_id/config undefined inside the worker on real hardware. The same class of arity gap calque#187 fixed for run/real/fleetrun, but spawn-run's own separate driver (internal/exec/spawnshard.go) had no equivalent. New SpawnCallable.MethodArgs carries the full non-self/cls arg list (ResolveSpawnCallables/ResolveOneSpawnCallable populate it via a new nonSelfCls helper). spawnManifestBody (cmd/calque/spawnrun.go) routes a 2+-arg callable through the SAME Starmap/MethodArgs splat mechanism .starmap() already uses in runner.py -- spawnArgsPayload was already building the right list payload, nothing downstream consumed it as a splat until now. A single-arg callable is byte-for-byte unchanged. Verified with a real subprocess execution test (TestSpawnDictDispatchMultiArgBindingEndToEnd) asserting both args' actual bound values through the real warm.Supervisor + runner.py, not just manifest-field inspection -- mirroring TestStarmapEndToEndDryRun's own proof strength.
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
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
SpawnCallable.MethodArgonly ever carried the FIRST non-self/cls parameter name — a callable like AI-Almanac'sforecasts_app.py'srun_forecast_inference(job_id, model_id, config)would (once .spawn() call site via a dynamic subscript (dict[key].spawn(...)) resolves to an empty target — spawnrun can't drive it #189/fix(parse): recognize .spawn() on a dict-of-functions Subscript (calque#189) #190/fix(parse): classify every spawn candidate as InvokeSpawn (calque#189 follow-up) #192's target-resolution fix landed) build a real shard, but silently leavemodel_id/configundefined inside the worker on real hardware. Same class of arity gap calque#187 fixed forrun/real/fleetrun;spawn-run's own separate driver had no equivalent.SpawnCallable.MethodArgscarries the full non-self/cls arg list (ResolveSpawnCallables/ResolveOneSpawnCallablepopulate it via a newnonSelfClshelper).spawnManifestBody(new, factored out ofrunSpawnShardfor testability) routes a 2+-arg callable through the SAMEStarmap/MethodArgssplat mechanism.starmap()already uses inrunner.py—spawnArgsPayloadwas already building the right list payload; nothing downstream consumed it as a splat until now. A single-arg callable (the common case) is byte-for-byte unchanged.Verification
Went beyond manifest-field inspection:
TestSpawnDictDispatchMultiArgBindingEndToEnddrives the real fixture'sbundle(job_id, config)through the actualwarm.Supervisor+runner.pysubprocess and asserts the actual returned values for both args — mirroringTestStarmapEndToEndDryRun's own proof strength (real execution, real result, not just "didn't crash" or "field is set"). Also extendedspawn_dict_dispatch.py's fixture to return both args so this assertion is possible.Files touched
internal/exec/spawnshard.go—SpawnCallable.MethodArgs,nonSelfCls, updatedResolveSpawnCallables/ResolveOneSpawnCallable.internal/exec/spawnshard_test.go— new unit tests forMethodArgspopulation (plain function + class method, self-stripping).cmd/calque/spawnrun.go— newspawnManifestBodypure function, wired intorunSpawnShard.cmd/calque/spawnrun_test.go(new) — unit tests forspawnManifestBody's splat-routing decision.cmd/calque/spawn_dict_dispatch_e2e_test.go— new real-execution end-to-end test.testdata/scripts/spawn_dict_dispatch.py— fixture extended to return both args.CHANGELOG.md.Test plan
go build ./... && go vet ./... && gofmt -l . && go test -count=1 ./...— all passgolangci-lint run ./...— 0 issuesruff check .— all checks passedTestSpawnDictDispatchMultiArgBindingEndToEnd— real subprocess execution, confirms bothjob_idandconfigbind to their real supplied values