Skip to content

Commit 7612e18

Browse files
committed
fix(cpp): filter skip fields out of metadata.common_input before op init (CI review)
CI diff-fuzz on this PR found a new go-vs-cpp divergence (seed 1784954760 divergence_000076) that the fixture-level fix did not cover. pine-go's SetMetadata (pine.go:159-168) filters skip control fields out of metadata.common_input before handing it to the operator via MetadataAware; the operator sees only business fields. My earlier #174 fix gated OperatorInput::common on the excluded_common set — the skip field's VALUE returns nil — but I never filtered the field NAME out of the list handed to operator init(). Ops that iterate cfg.metadata.common_input to build a salt/hash (reorder_shuffle_by_salt loops over common_inputs_ in execute()) then see an extra entry: Go common_input=[count] salt = "17|" cpp common_input=[count,_skip] salt = "17||" Same downstream cascade as #174: different salt → different shuffle order → cascading downstream item order divergence. Fix: at engine construction, filter cfg.skip out of op_cfg.metadata.common_input before calling op->init(cfg). Matches Go's SetMetadata filter step and applies to every operator uniformly, not just shuffle. DAG dependency inference sees the unfiltered read set via CommonReadFields at plan time, so wiring is unaffected. Verified: the failing CI seed 1784954760 100 rounds is now 100/0; seed 20260725 500 rounds also 100/0; nightly divergence #174/#177 regressions still pass; cross-validate 3/9 → 98/98 + 91/91.
1 parent ca2baf0 commit 7612e18

1 file changed

Lines changed: 22 additions & 0 deletions

File tree

pine-cpp/src/runtime/engine.cpp

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -355,6 +355,28 @@ Engine::Engine(Config config, EngineOptions options) : config_(std::move(config)
355355
op_cfg.type_name + "\"");
356356
}
357357
auto instance = entry->factory();
358+
// Filter skip control fields out of metadata.common_input before init,
359+
// matching pine-go's SetMetadata (pine.go: commonIn = filterOutField
360+
// over opCfg.Skip). Operators reading cfg.metadata.common_input (e.g.
361+
// reorder_shuffle_by_salt building its salt) must see only the
362+
// business fields; the skip control field's value is still gated to
363+
// nil at read time via OperatorInput::common (issue #174), but the
364+
// field NAME must not appear in the list either — otherwise ops that
365+
// iterate the list to build a salt/hash produce an extra "|" slot on
366+
// C++ where Go emits nothing, diverging the resulting order across
367+
// runtimes. DAG dependency inference already sees the unfiltered
368+
// read set via Metadata::CommonReadFields at plan time.
369+
if (!op_cfg.skip.empty()) {
370+
std::set<std::string> skip_set(op_cfg.skip.begin(), op_cfg.skip.end());
371+
std::vector<std::string> filtered;
372+
filtered.reserve(op_cfg.metadata.common_input.size());
373+
for (const auto& f : op_cfg.metadata.common_input) {
374+
if (!skip_set.count(f)) {
375+
filtered.push_back(f);
376+
}
377+
}
378+
op_cfg.metadata.common_input = std::move(filtered);
379+
}
358380
instance->init(op_cfg);
359381
// Optional-interface injection. Cross-runtime relative order (see
360382
// llmdoc/architecture/dag-engine.md invariant 11): Logger -> Metrics ->

0 commit comments

Comments
 (0)