diff --git a/CHANGELOG.md b/CHANGELOG.md index dac357a..4a8aa35 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,26 @@ per [semver.org](https://semver.org/#spec-item-4). ### Fixed +- calque#189's fix was necessary but not sufficient: `SpawnCallSites`' + candidate expansion made a dict-subscript `.spawn()` call SITE visible, + but the candidate callable's own `ir.Function.Invoke` was still never + set to `InvokeSpawn` (it stayed the empty string) — `invocationKinds`' + `consider()` call for the "spawn" case was still keyed on the empty + `ic.Target`, not the candidates. `ResolveSpawnCallables` + (`internal/exec/spawnshard.go`) only ever finds a callable whose OWN + `Invoke` is `InvokeSpawn`, so a real end-to-end + `Parse -> ResolveSpawnCallables -> BuildSpawnManifests` run still + produced ZERO shards for a script using this idiom, silently — the exact + failure #189 was filed to prevent, one layer deeper. Found by writing a + synthesized end-to-end test that actually wires the pipeline together + (`cmd/calque/spawn_dict_dispatch_e2e_test.go`) instead of only testing + `SpawnCallSites` in isolation — the defect lived in the gap BETWEEN two + already-individually-tested layers, invisible to either one's own unit + tests. `invocationKinds` now classifies every candidate as `InvokeSpawn` + when the call site's `Target` is empty but `Candidates` is populated. + Also surfaced (filed separately, not fixed here — a distinct, pre- + existing limitation): calque#191, `spawn-run` only ever binds a spawned + callable's FIRST positional arg regardless of how many it real has. - A `.spawn()` call whose receiver is a Subscript on a module-level dict-of-functions selected by a runtime key (e.g. AI-Almanac's `forecasts_app.py`'s `SEASON_BUNDLE_FNS[_model_env(model_id)].spawn(...)`) diff --git a/cmd/calque/spawn_dict_dispatch_e2e_test.go b/cmd/calque/spawn_dict_dispatch_e2e_test.go new file mode 100644 index 0000000..76de09e --- /dev/null +++ b/cmd/calque/spawn_dict_dispatch_e2e_test.go @@ -0,0 +1,95 @@ +package main + +import ( + "context" + "os/exec" + "path/filepath" + "testing" + + calexec "github.com/spore-host/calque/internal/exec" + "github.com/spore-host/calque/internal/ir" + "github.com/spore-host/calque/internal/leak" + "github.com/spore-host/calque/internal/parse" +) + +// TestSpawnDictDispatchEndToEndProducesRealShards is the end-to-end +// regression guard for calque#189: it wires Parse -> ResolveSpawnCallables +// -> SpawnCallSitesReport -> BuildSpawnManifests together exactly as +// spawnRunFromScript (spawnrun.go) does, against a real fixture whose +// .spawn() receiver is a dict-of-functions Subscript (mirroring +// AI-Almanac's forecasts_app.py exactly). +// +// This test exists because the original #189 fix (SpawnCallSites' +// candidate expansion) was necessary but NOT sufficient on its own: a +// diagnostic run of this exact pipeline, built to answer "does spawn-run +// actually produce a shard for this script," found that +// ResolveSpawnCallables returned ZERO callables even after that fix — the +// picked function's own ir.Function.Invoke was still "" (not +// ir.InvokeSpawn), because invocationKinds' consider() call for the +// "spawn" case was still keyed on the empty ic.Target, never the +// candidates. SpawnCallSites (the call-SITE side) and +// ResolveSpawnCallables (the callable-DEFINITION side) are two +// independently-tested layers; the defect lived in the gap BETWEEN them, +// invisible to either layer's own isolated unit tests. Only assembling the +// real pipeline end-to-end — not just asserting on SpawnCallSites' return +// value in isolation — surfaced it. +func TestSpawnDictDispatchEndToEndProducesRealShards(t *testing.T) { + if _, err := exec.LookPath("uv"); err != nil { + t.Skip("uv not on PATH; skipping pyast contract test") + } + if _, err := exec.LookPath("python3"); err != nil { + t.Skip("python3 not on PATH; skipping pyast contract test") + } + setPyastDirEnv(t) + script, err := filepath.Abs("../../testdata/scripts/spawn_dict_dispatch.py") + if err != nil { + t.Fatal(err) + } + ctx := context.Background() + rep := &leak.Report{} + runner, args := parse.DefaultRunner(pyastDir()) + + app, err := parse.Parse(ctx, script, rep, runner, args...) + if err != nil { + t.Fatalf("Parse: %v", err) + } + + var bundle ir.Function + found := false + for _, f := range app.Functions { + if f.Name == "bundle" { + bundle, found = f, true + } + } + if !found { + t.Fatal(`app.Functions has no "bundle" — fixture regressed`) + } + if bundle.Invoke != ir.InvokeSpawn { + t.Errorf(`bundle.Invoke = %q, want %q — a dict-subscript .spawn() candidate must classify the CANDIDATE, not the empty target`, bundle.Invoke, ir.InvokeSpawn) + } + + callables := calexec.ResolveSpawnCallables(app) + if len(callables) != 1 || callables[0].Key != "bundle" { + t.Fatalf("ResolveSpawnCallables = %+v, want exactly one callable keyed \"bundle\"", callables) + } + + sites, err := parse.SpawnCallSitesReport(ctx, script, rep, runner, args...) + if err != nil { + t.Fatalf("SpawnCallSitesReport: %v", err) + } + if len(sites) != 1 || sites[0].Target != "bundle" { + t.Fatalf("SpawnCallSitesReport = %+v, want exactly one site targeting \"bundle\"", sites) + } + + callSites := make([]calexec.SpawnCallSite, len(sites)) + for i, s := range sites { + callSites[i] = calexec.SpawnCallSite{Target: s.Target, Args: s.Args} + } + shards := calexec.BuildSpawnManifests(callables, callSites, "s3://bucket/base", "s3://bucket/artifacts") + if len(shards) != 1 { + t.Fatalf("BuildSpawnManifests produced %d shard(s), want exactly 1 — this is the real, end-user-visible failure mode #189 exists to prevent: zero shards means `calque spawn-run` silently does nothing for this script. shards=%+v", len(shards), shards) + } + if shards[0].Key != "bundle" { + t.Errorf("shards[0].Key = %q, want %q", shards[0].Key, "bundle") + } +} diff --git a/internal/parse/parse.go b/internal/parse/parse.go index aa17634..1a38edd 100644 --- a/internal/parse/parse.go +++ b/internal/parse/parse.go @@ -598,6 +598,29 @@ func invocationKinds(out pyOut, script string, rep *leak.Report) (map[string]ir. // still not EXECUTED (§18 keeps calque block-and-wait-only). The leak // reflects that shift: "we know what this is" rather than "deferred, // unclassified." + // + // calque#189 follow-up: an empty ic.Target (dict-of-functions + // Subscript receiver, e.g. SEASON_BUNDLE_FNS[key].spawn(...)) + // must classify EVERY Candidate as ir.InvokeSpawn, not the + // empty string — SpawnCallSites already expands the CALL SITE + // side into one entry per candidate, but ResolveSpawnCallables + // (internal/exec/spawnshard.go) only ever finds a callable + // whose OWN ir.Function.Invoke is InvokeSpawn in the first + // place. Without this, a real end-to-end + // parse->ResolveSpawnCallables->BuildSpawnManifests run + // produces ZERO shards for a script using this idiom — found + // via a synthesized end-to-end test that actually wired the + // pieces together, not caught by testing SpawnCallSites in + // isolation (the bug this fixes is a gap BETWEEN two already- + // individually-tested layers, not inside either one). + if ic.Target == "" && len(ic.Candidates) > 0 { + for _, c := range ic.Candidates { + consider(ic.Entrypoint, c, ir.InvokeSpawn) + } + rep.Addf(leak.PrimMap, leak.KindSemanticGap, script, ic.Lineno, + "spawn(...) on a dict-of-functions Subscript selected by a runtime key; classifying every candidate (%v) as InvokeSpawn since the real runtime selection isn't statically resolvable (calque#189)", ic.Candidates) + continue + } consider(ic.Entrypoint, ic.Target, ir.InvokeSpawn) rep.Addf(leak.PrimMap, leak.KindSemanticGap, script, ic.Lineno, "%s.spawn(...): classified but not executed — block-and-wait fan-out over distinct spawned callables is deferred per §18 (calque#97 tracks the driver)", leafName(ic.Target)) diff --git a/internal/parse/parse_test.go b/internal/parse/parse_test.go index 7aa07a5..b68f9f8 100644 --- a/internal/parse/parse_test.go +++ b/internal/parse/parse_test.go @@ -902,6 +902,40 @@ func TestInvocationKindsPartitionsByEntrypoint(t *testing.T) { } } +// TestInvocationKindsClassifiesSpawnCandidatesNotEmptyTarget (calque#189 +// follow-up): a spawn call site with an empty Target but real Candidates +// (a dict-of-functions Subscript receiver) must classify EVERY candidate +// as ir.InvokeSpawn — NOT the empty string. Found via an end-to-end test +// that actually wired Parse->ResolveSpawnCallables->BuildSpawnManifests +// together (see internal/exec's own TestBuildSpawnManifestsForDictDispatch +// fixture-driven test): SpawnCallSites' own candidate expansion (the +// original #189 fix) was necessary but not sufficient — without ALSO +// fixing invocationKinds here, ResolveSpawnCallables never marks the +// candidate callable as InvokeSpawn in the first place, so +// BuildSpawnManifests' byTarget lookup has a call site with nothing to +// shard against and silently produces ZERO shards. This is the gap +// BETWEEN two already-individually-tested layers, not a bug inside either +// one — exactly why an end-to-end test caught it and isolated unit tests +// on SpawnCallSites alone did not. +func TestInvocationKindsClassifiesSpawnCandidatesNotEmptyTarget(t *testing.T) { + out := pyOut{ + InvokeCalls: []pyInvokeCall{ + {Target: "", Kind: "spawn", Candidates: []string{"bundle_a", "bundle_b"}}, + }, + } + rep := &leak.Report{} + whole, _ := invocationKinds(out, "s.py", rep) + if whole["bundle_a"] != ir.InvokeSpawn { + t.Errorf(`whole["bundle_a"] = %q, want %q`, whole["bundle_a"], ir.InvokeSpawn) + } + if whole["bundle_b"] != ir.InvokeSpawn { + t.Errorf(`whole["bundle_b"] = %q, want %q`, whole["bundle_b"], ir.InvokeSpawn) + } + if _, ok := whole[""]; ok { + t.Errorf(`whole must not classify the empty string as invoked; got %+v`, whole) + } +} + // TestParseEntrypointScopedInvokes (calque#98): the fixture's two entrypoints // each invoke a wholly DIFFERENT callable (do_train -> Trainer.train_step via // .map(), do_evaluate -> evaluate via .remote()) — app.EntrypointInvokes must