Conversation
There was a problem hiding this comment.
Pull request overview
Refactors the deterministic scenario planning/simulation pipeline to derive scenario packs from situation artifacts (with planning templates + trace artifacts), strengthens situation/signal validation, and removes the legacy NL scenario pack compiler/flags.
Changes:
- Add YAML-driven planning/signals templates and new planning/prior/space helpers; enforce deterministic A/B/C ordering.
- Extend situation analysis to normalize/validate rich “signal” schemas and tighten situation artifact validation.
- Remove NL pack compilation paths/CLI flags; update CLI + tests to use
scenario_pack.jsoninputs and persist new trace artifacts (planning query, prior snapshot, actor derivation).
Reviewed changes
Copilot reviewed 52 out of 52 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/test_situation_signals.py | Adds unit coverage for signal-template injection + signal schema normalization/validation. |
| tests/unit/test_situation_cli_pack_id.py | Updates expected default paths for situation/scenario artifacts. |
| tests/unit/test_scenario_pack_inference.py | Removes tests tied to deleted NL slot inference. |
| tests/test_smoke.py | Updates smoke tests to run simulate/compare directly from a minimal scenario pack file. |
| tests/integration/test_strategic_freedom_conditions.py | Updates integration test to run compare from a generated scenario pack. |
| tests/integration/test_situation_pipeline.py | Updates integration expectations for new default output locations and trace artifacts. |
| tests/integration/test_scenario_planning_stability.py | Adds integration coverage for stable prior ordering + trace refs (currently has fixture issues). |
| tests/integration/test_nl_scenario_compilation.py | Reworks test to assert removed NL compilation path exits with error. |
| tests/integration/test_deterministic_scope_guardrails.py | Renames test to reflect removed local NL compile flag behavior. |
| tests/integration/test_deterministic_scenario_run.py | Updates integration tests to simulate/compare using a scenario pack file. |
| tests/integration/test_actor_derivation_trace_artifact.py | Adds integration test ensuring actor derivation trace is persisted and referenced. |
| tests/contract/test_scenario_prior_snapshot_contract.py | Adds contract test for prior snapshot normalization and A/B/C preservation. |
| src/omen/scenario/validator.py | Adds deterministic scenario ordering validation and strict situation signal schema checks. |
| src/omen/scenario/template_loader.py | Adds loader/validator for planning template YAML. |
| src/omen/scenario/space.py | Adds deterministic planning-query builder (graph + space inputs + priors). |
| src/omen/scenario/prior.py | Adds prior snapshot builder + normalization helper. |
| src/omen/scenario/planner.py | Introduces scenario planning orchestrator that writes planning/prior traces and builds ontology. |
| src/omen/scenario/pack_compiler.py | Removes NL scenario compilation module entirely. |
| src/omen/scenario/ontology_models.py | Renames scenario node model and adds planning/prior refs to ontology slice schema. |
| src/omen/scenario/models.py | Adds Pydantic models for planning templates, planning query, and prior snapshots. |
| src/omen/scenario/loader.py | Removes NL compile entrypoint; adds helpers to resolve situation/scenario refs by pack_id. |
| src/omen/ingest/synthesizer/services/situation.py | Loads signal schema template and normalizes signals into mechanism-bearing schema; passes planning inputs into scenario decomposition prompt. |
| src/omen/ingest/synthesizer/builders/situation.py | Adds planning/prior refs into scenario ontology builder and updates import location. |
| src/omen/cli/situation.py | Updates CLI defaults/paths, adds --actor, uses new planner + ref resolution. |
| src/omen/cli/main.py | Removes --deterministic-nl-json flags; passes planned scenarios + derivation trace path into simulate/compare. |
| src/omen/cli/case.py | Integrates actor derivation artifacts/traces into deterministic simulate/compare runs. |
| src/omen/cli/actor.py | Removes analyze actor compile-pack subcommand. |
| src/omen/analysis/actor/report_writer.py | Adds writer for actor derivation artifacts. |
| src/omen/analysis/actor/insight.py | Improves missing-evidence message to include scenario key. |
| src/omen/analysis/actor/formation.py | Incorporates planned scene objective/constraints/tradeoffs into dimension rationale. |
| src/omen/analysis/actor/derivation.py | Adds actor derivation + strategic freedom condition derivation helpers. |
| src/omen/analysis/actor/derivation_trace.py | Adds derivation trace and derivation artifact builders. |
| src/omen/analysis/actor/comparability.py | Adds executed-order comparability checks. |
| src/omen/init.py | Bumps package version to 0.1.1. |
| README.zh.md | Adds additional project badges. |
| README.md | Adds additional project badges. |
| pyproject.toml | Bumps project version to 0.1.1. |
| docs/guides/situation-analysis.md | Adds a situation analysis guide (currently references legacy default paths/filenames). |
| data/scenarios/sap_v1/traces/prior_snapshot.json | Adds sample prior snapshot trace artifact. |
| data/scenarios/sap_v1/traces/planning_query.json | Adds sample planning query trace artifact. |
| data/scenarios/sap_v1/scenario_pack.md | Adds sample scenario pack markdown output. |
| data/scenarios/sap_v1/scenario_pack.json | Adds sample scenario pack JSON output. |
| data/scenarios/sap_v1/sap_reltio_acquisition_situation.md | Updates sample situation brief output. |
| data/scenarios/sap_v1/sap_reltio_acquisition_situation.json | Updates sample situation artifact with normalized signals/space seeds. |
| data/scenarios/sap_v1/sap_reltio_acquisition_generation.json | Updates sample generation trace metrics/confidence. |
| data/scenarios/nokia_v1/nokia-elop-2010_situation.md | Adds sample Nokia situation brief output. |
| data/scenarios/nokia_v1/nokia-elop-2010_generation.json | Adds sample Nokia generation trace output. |
| config/templates/signals.yaml | Adds YAML schema contract for normalized situation signals. |
| config/templates/planning.yaml | Adds YAML planning template with slot policy and default priors. |
| config/prompts/base.yaml | Strengthens LLM prompt contract for signal extraction + planning inputs. |
| cases/situations/sap_reltio_acquisition.md | Updates sample situation case content and capture date. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| pack_id = "strategic_actor_nokia_v1" | ||
| situation_path = tmp_path / "data" / "scenarios" / pack_id / "situation.json" | ||
| _write_situation_artifact(situation_path) |
There was a problem hiding this comment.
The test writes the situation artifact to data/scenarios/<pack_id>/situation.json, but the CLI resolves pack-id inputs to data/scenarios/<pack_id>/generation/situation.json (see resolve_situation_artifact_ref). As written, omen scenario --situation <pack_id> will fail with "situation artifact not found". Update the test to write the artifact to the resolved default location (or pass the explicit JSON path to --situation).
| }, | ||
| "signals": [{"name": "ecosystem fragmentation"}, {"name": "cash pressure"}], | ||
| "tech_space_seed": [], | ||
| "market_space_seed": [], | ||
| "uncertainty_space": {"overall_confidence": 0.5}, | ||
| "source_trace": [{"source_path": "cases/situations/nokia-elop-2010.md"}], |
There was a problem hiding this comment.
The situation artifact payload in this test uses signals entries with only {name: ...}. The updated validate_situation_artifact_or_raise requires each signal to include fields like id, domain, strength, direction, mapped_targets, cascade_rules/no_cascade_reason, market_constraints, and mechanism_note, so load_situation_artifact() will reject this JSON before planning runs. Adjust the fixture payload to conform to the required signal schema (or generate via analyze_situation_document).
| - `data/scenarios/sap_v1/sap_reltio_acquisition_situation.json` | ||
| - `data/scenarios/sap_v1/sap_reltio_acquisition_situation.md` | ||
| - `data/scenarios/sap_v1/sap_reltio_acquisition_generation.json` |
There was a problem hiding this comment.
This guide still references the old default analyze outputs like <stem>_situation.json and <stem>_generation.json. The CLI now defaults to data/scenarios/<pack_id>/generation/situation.json and generation/generation.json (and scenario output to scenario_pack.json). Update the documented output paths accordingly so the guide matches current behavior.
| - `data/scenarios/sap_v1/sap_reltio_acquisition_situation.json` | |
| - `data/scenarios/sap_v1/sap_reltio_acquisition_situation.md` | |
| - `data/scenarios/sap_v1/sap_reltio_acquisition_generation.json` | |
| - `data/scenarios/sap_v1/scenario_pack.json` | |
| - `data/scenarios/sap_v1/sap_reltio_acquisition_situation.md` | |
| - `data/scenarios/sap_v1/generation/situation.json` | |
| - `data/scenarios/sap_v1/generation/generation.json` |
| ## 5. 下一步:从 Situation 到 Scenario | ||
|
|
||
| 拿到 situation artifact 后,执行场景分解: | ||
|
|
||
| ```bash | ||
| omen scenario --situation data/scenarios/sap_v1/sap_reltio_acquisition_situation.json | ||
| ``` | ||
|
|
||
| 随后可继续: | ||
|
|
||
| ```bash | ||
| omen simulate --scenario data/scenarios/sap_v1/sap_reltio_acquisition.json | ||
| ``` |
There was a problem hiding this comment.
The example commands in this section use legacy filenames (*_situation.json and *.json for scenario packs). With the new defaults, the scenario command can take a pack_id or .../generation/situation.json, and simulate should point at .../scenario_pack.json. Please update the commands so they work with the current CLI defaults.
| def _normalize_signal_strength(value: Any) -> str: | ||
| if isinstance(value, (int, float)): | ||
| return _clamp_01(float(value)) | ||
|
|
||
| text = str(value or "").strip().lower() | ||
| if text in {"low", "weak"}: | ||
| return 0.3 | ||
| if text in {"high", "strong"}: | ||
| return 0.8 | ||
| if text in {"medium", "mid", "moderate"}: | ||
| return 0.5 | ||
|
|
||
| parsed = _to_float(value) | ||
| if parsed is None: | ||
| return 0.5 | ||
| return _clamp_01(parsed) |
There was a problem hiding this comment.
_normalize_signal_strength is annotated to return str, but it consistently returns floats (and _clamp_01(...) also appears to return a float). This will trip type checkers and makes the API misleading. Change the return annotation to float (and consider aligning other signal-normalization type hints like _normalize_signal_targets).
| if args.pack_id: | ||
| pack_id = str(args.pack_id) | ||
| elif "/" not in str(args.situation) and not str(args.situation).endswith(".json"): | ||
| pack_id = str(args.situation) | ||
| else: | ||
| pack_id = _derive_pack_id_from_situation_artifact(situation_artifact, situation_path) |
There was a problem hiding this comment.
Pack-id inference here checks for '/' in args.situation, which is not portable on Windows paths (backslashes) and can misclassify inputs. Consider using Path(args.situation) semantics instead (e.g., check Path(...).suffix == '.json' or whether the ref resolves to an existing file) so pack-id vs path detection is platform-independent.
No description provided.