Round-2 verification fixes: nested SVAR errors, contract hardening, population-claims sweep

.github/workflows/contract.yml

@@ -23,6 +23,6 @@ jobs:
       - name: Install with full models
         run: uv pip install --system -e ".[models]" pytest
       - name: Contract tests (failures fail, never skip)
-        run: pytest tests/test_contract.py --runslow -q
+        run: pytest tests/test_contract.py tests/test_og.py::test_og_build_policy_real --runslow -q
         env:
           MACROMOD_REQUIRE_PE: "1"

.gitignore

@@ -4,3 +4,9 @@ integration/src/*.egg-info/
 __pycache__/
 *.pyc
 .DS_Store
+
+# Local env and deployment link files — never commit (a Vercel OIDC token
+# was nearly published this way)
+.env*.local
+.env.local
+.vercel/

README.md

@@ -169,7 +169,7 @@ non-real numbers as illustrative.
 - [x] Local MCP server (`python -m macromod.mcp_server`)
 - [x] Hosted MCP server (`https://policyengine--macromod-mcp-serve.modal.run/mcp`, auto-deployed by CI)
 - [x] OG-UK steady-state scoring (`macromod score --model og` / `macromod og-score`, local only)
-- [ ] Population-level PolicyEngine reform scoring
+- [x] Population-level PolicyEngine reform scoring (`population_reform_impact`, hosted and local)
 - [ ] Additional macroeconomic model classes
 - See [#1](https://github.com/PolicyEngine/macro/issues/1) — Rust port of the solver core
 

connect/index.html

@@ -750,9 +750,9 @@ <h4>Sector output, long-run</h4>
         tax, National Insurance, Universal Credit, SNAP, EITC, and the rest —
         for the household you describe. Population-level analysis over
         representative microdata is in the same package; population-level
-        reform scoring is available locally (it needs large private microdata),
-        but is not on the hosted MCP server — the hosted PolicyEngine tools are
-        household-level only.
+        reform scoring runs on the hosted MCP server too (the private
+        microdata credential is provisioned server-side) and locally with a
+        HUGGING_FACE_TOKEN.
       </p>
 
       <div class="dials">

index.html

@@ -137,14 +137,14 @@ <h3>Bank of England structural VAR model</h3>
         <span class="strategy-paper-link mono">read the model →</span>
       </a>
       <a class="strategy-card" href="/pe/">
-        <header><span class="card-id mono">pe-microsim</span><span class="tag-row"><span class="tag tag-hosted">hosted: household</span><span class="tag tag-planned">population planned</span></span></header>
+        <header><span class="card-id mono">pe-microsim</span><span class="tag-row"><span class="tag tag-hosted">hosted: household</span><span class="tag tag-hosted">hosted: population</span></span></header>
         <h3>PolicyEngine tax-benefit microsimulation</h3>
         <p>
           The micro member of the suite: PolicyEngine's tax-benefit
           microsimulation package for the UK and US — the same engine behind
           policyengine.org. Computes a specific family's taxes, benefits, and
           net income under current law or a reform, at person and household
-          resolution; population-level reform scoring is planned.
+          resolution; population-level reform scoring runs hosted and locally.
         </p>
         <span class="strategy-paper-link mono">read the model →</span>
       </a>

integration/README.md

@@ -13,7 +13,7 @@ one CLI and one MCP server:
   microdata (UK data is private: set `HUGGING_FACE_TOKEN`; the hosted
   deployment provisions it server-side).
 - **OG-UK** (`oguk`, optional/local-only): overlapping-generations
-  steady-state scoring through `score_reform --model og`.
+  steady-state scoring through `macromod score --model og`.
 
 `score_reform` is the one reform vocabulary across the suite: the same flat
 `{parameter_path: value}` dict as the microsimulation tools, dispatched to a
@@ -36,11 +36,14 @@ pip install "macromod[models] @ git+https://github.com/PolicyEngine/macro#subdir
 
 A shorter `pip install macromod` will come with PyPI publication.
 
-For development, install local checkouts editable into a fresh env:
+For development, install with the full model set (policyengine included via
+the `[models]` extra), overriding the two model packages with local editable
+checkouts; OG-UK is optional and local-only:
 
 ```bash
-uv venv && uv pip install -e ./integration \
-    -e ../obr-macroeconomic-model -e ../boe-var-model pytest
+uv venv && uv pip install -e "./integration[models]" pytest \
+    -e ../obr-macroeconomic-model -e ../boe-var-model
+uv pip install "oguk @ git+https://github.com/PSLmodels/OG-UK"  # optional, for --model og
 ```
 
 ## CLI

integration/modal_app.py

@@ -9,8 +9,9 @@
 ASGI app at  https://policyengine--macromod-mcp-serve.modal.run/mcp
 
 Both model repos resolve their data files relative to their own repo root
-(`Path(__file__)`-relative), and `macromod.core` hardcodes
-/Users/janansadeqian/boe-var-model. We therefore bake the repos into the
+(`Path(__file__)`-relative); `macromod.core`'s svar_summary falls back to a
+checkout via MACROMOD_BOE_VAR_REPO only when boe_var is absent (it is
+installed here, so the fallback never fires). We bake the repos into the
 image at the SAME absolute paths and `pip install -e` them, so every path
 resolves in the container with zero patching:
   - obr_macro:  /Users/janansadeqian/obr-macroeconomic-model  (+ data/)

integration/src/macromod/cli.py

@@ -141,11 +141,19 @@ def summary(as_json):
     res = core.svar_summary()
     if set(res) == {"error"}:
         raise click.ClickException(res["error"])
+    rep = res.get("replication", {})
+    fr_check = res.get("forecast_revision", {})
+    if "error" in rep and "error" in fr_check:
+        raise click.ClickException(
+            "no parseable SVAR results — replication: "
+            f"{rep['error']}; forecast revision: {fr_check['error']}"
+        )
     if as_json:
         _emit_json(res)
         return
-    rep = res.get("replication", {})
     click.echo("Replication (results/summary.md)")
+    if "error" in rep:
+        click.echo(f"  error: {rep['error']}", err=True)
     for ln in rep.get("metadata", []):
         click.echo(f"  {ln}")
     fevd = rep.get("fevd_1yr_headline", [])
@@ -154,6 +162,8 @@ def summary(as_json):
         click.echo(_table(fevd, list(fevd[0].keys())))
     fr = res.get("forecast_revision", {})
     click.echo("\nForecast-revision exercise (results/forecast_summary.md)")
+    if "error" in fr:
+        click.echo(f"  error: {fr['error']}", err=True)
     for ln in fr.get("metadata", []):
         click.echo(f"  {ln}")
     signs = fr.get("latest_shock_signs", [])

integration/tests/test_contract.py

@@ -12,19 +12,32 @@
 e.g. the Modal image.
 """
 
+import importlib
 import os
 
 import pytest
 
 from macromod import core
 
 
+def _import_or_require(modname: str):
+    """Import an upstream module; in the dedicated contract job
+    (MACROMOD_REQUIRE_PE=1) a missing/broken install FAILS instead of
+    skipping — a contract run that silently skips is vacuous."""
+    require = os.environ.get("MACROMOD_REQUIRE_PE") == "1"
+    try:
+        return importlib.import_module(modname)
+    except Exception as e:
+        msg = f"{modname} not importable: {type(e).__name__}: {e}"
+        pytest.fail(msg) if require else pytest.skip(msg)
+
+
 # ---------------------------------------------------------------------------
 # boe_var (fast)
 # ---------------------------------------------------------------------------
 
 def test_svar_headline_columns_exist_upstream():
-    data = pytest.importorskip("boe_var.data")
+    data = _import_or_require("boe_var.data")
     for col in (core._COL_CPI, core._COL_GDP):
         assert col in data.COLUMNS, (
             f"boe_var.data.COLUMNS no longer contains {col!r}; "
@@ -36,7 +49,7 @@ def test_svar_identified_schema_aligned():
     """WORLD_SHOCKS + UK_SHOCKS is the canonical schema the adapter uses;
     SHOCK_NAMES must stay positionally aligned with it (an upstream reorder
     must fail here, not silently shift UK shock probabilities)."""
-    analysis = pytest.importorskip("boe_var.analysis")
+    analysis = _import_or_require("boe_var.analysis")
     identified = sorted(analysis.WORLD_SHOCKS + analysis.UK_SHOCKS)
     assert len(identified) == 6 and len(set(identified)) == 6
     assert all(0 <= j < len(analysis.SHOCK_NAMES) for j in identified)
@@ -49,6 +62,14 @@ def test_svar_identified_schema_aligned():
         if j not in identified
     ]
     assert all(n.startswith("Unident") for n in unidentified), unidentified
+    # Semantic alignment, not just structure: a swap of e.g. UK demand/supply
+    # labels would silently relabel probabilities — pin the exact names.
+    assert tuple(
+        analysis.SHOCK_NAMES[j] for j in analysis.WORLD_SHOCKS
+    ) == ("World demand", "World energy", "World supply")
+    assert tuple(
+        analysis.SHOCK_NAMES[j] for j in analysis.UK_SHOCKS
+    ) == ("UK demand", "UK supply", "UK mon. pol.")
 
 
 # ---------------------------------------------------------------------------

integration/tests/test_core.py

@@ -50,7 +50,7 @@ def test_svar_forecast_and_cache():
 # ---------------------------------------------------------------------------
 
 def test_pe_list_common_parameters():
-    params = core.pe_list_common_parameters()
+    params = core.pe_list_common_parameters(resolve=False)
     assert len(params) >= 8
     for p in params:
         assert {"country", "path", "description", "unit"} <= set(p)
@@ -165,3 +165,17 @@ def test_cli_summary_without_boe_var_errors_actionably(monkeypatch):
     res = CliRunner().invoke(main, ["summary"])
     assert res.exit_code != 0
     assert "MACROMOD_BOE_VAR_REPO" in res.output
+
+
+def test_cli_summary_env_checkout_missing_files_errors(monkeypatch, tmp_path):
+    """A configured fallback whose files are missing must error, not print
+    empty headings and exit 0 (round-2 review, new finding 1)."""
+    from click.testing import CliRunner
+
+    from macromod.cli import main
+
+    _block_boe_var(monkeypatch)
+    monkeypatch.setenv("MACROMOD_BOE_VAR_REPO", str(tmp_path))
+    res = CliRunner().invoke(main, ["summary"])
+    assert res.exit_code != 0
+    assert "no parseable SVAR results" in res.output

integration/tests/test_remote_mcp.py

@@ -26,7 +26,9 @@
 
 EXPECTED_TOOLS = {
     "score_reform", "obr_shock", "list_reform_variables", "forecast_uk",
-    "latest_shocks", "model_summary",
+    "latest_shocks", "model_summary", "calculate_household",
+    "household_reform_impact", "list_reform_parameters",
+    "population_reform_impact",
 }
 
 

integration/tests/test_wiring.py

@@ -254,3 +254,39 @@ def test_core_obr_extreme_shock_is_wellformed():
     res = core.obr_shock(var="CGG", shock=1_000_000, periods=2)
     assert res["periods"] == 2 and len(res["results"]) >= 2
     json.dumps(res)
+
+
+def test_cli_obr_shock_closure_tristate(runner, monkeypatch):
+    """Omitted --investment-closure must reach core as None (per-variable
+    default), not False — the TCPRO zero-effects footgun (review #15.1)."""
+    seen = []
+
+    def fake_obr_shock(**kwargs):
+        seen.append(kwargs["investment_closure"])
+        return {"name": "x", "var": "TCPRO", "shock": -0.05, "periods": 1,
+                "investment_closure": True, "results": [],
+                "cumulative_delta_gdp_bn_over_shock_periods": 0.0,
+                "peak_pct_gdp": 0.0, "peak_period": "2025Q1"}
+
+    monkeypatch.setattr(core, "obr_shock", fake_obr_shock)
+    for args, expected in [
+        (["obr-shock", "--var", "TCPRO", "--shock", "-0.05", "--json"], None),
+        (["obr-shock", "--var", "TCPRO", "--shock", "-0.05",
+          "--investment-closure", "--json"], True),
+        (["obr-shock", "--var", "TCPRO", "--shock", "-0.05",
+          "--no-investment-closure", "--json"], False),
+    ]:
+        res = runner.invoke(main, args)
+        assert res.exit_code == 0, res.output
+        assert seen[-1] is expected
+
+
+def test_cli_wrong_shaped_reform_is_clean_error(runner):
+    """Valid JSON of the wrong shape ('[]', '{}') must be a Click error,
+    never a traceback (review #15.4)."""
+    for cmd in (["score", "--model", "og"], ["og-score"]):
+        for bad in ("[]", "{}"):
+            res = runner.invoke(main, cmd + ["--reform", bad])
+            assert res.exit_code != 0
+            assert "Traceback" not in res.output
+            assert "non-empty" in res.output

pe/index.html

@@ -212,7 +212,7 @@ <h2>What it answers — and what it doesn't yet.</h2>
       </p>
       <ul>
         <li>It is a <strong>static microsimulation</strong> at heart — reforms are modelled statically, with optional post-hoc behavioural responses (e.g. labour supply elasticities). No general equilibrium: that second act is exactly what the macro members of the suite add.</li>
-        <li><strong>PolicyEngine Macro integration status:</strong> the household tools — computing one specific family's taxes, benefits, and net income under current law or a reform — are hosted and live on the MCP server (and the <code>macromod</code> CLI): <code>calculate_household</code>, <code>household_reform_impact</code>, and <code>list_reform_parameters</code>. Representative-population reform scoring (aggregates, deciles, poverty, and inequality over whole microdata, via <code>ensure_datasets</code> + <code>Simulation</code>) is planned but not yet connected to the hosted service: the enhanced-FRS microdata lives in a private Hugging Face repository and the downloads are large, so it does not yet fit the zero-auth, scale-to-zero deployment.</li>
+        <li><strong>PolicyEngine Macro integration status:</strong> the household tools — computing one specific family's taxes, benefits, and net income under current law or a reform — are hosted and live on the MCP server (and the <code>macromod</code> CLI): <code>calculate_household</code>, <code>household_reform_impact</code>, and <code>list_reform_parameters</code>. Representative-population reform scoring (<code>population_reform_impact</code>: budgetary impact, deciles, winners/losers over whole microdata) is hosted too — the private enhanced-FRS credential is provisioned server-side — and runs locally with a <code>HUGGING_FACE_TOKEN</code>.</li>
         <li>Two countries: UK and US.</li>
       </ul>
     </div>