From 64bb83f7499e4e35c61d0dfe81445a31b6fecc7c Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Wed, 15 Apr 2026 09:15:16 +0200 Subject: [PATCH] =?UTF-8?q?fix(simulation):=20address=20PR=20#129=20review?= =?UTF-8?q?=20=E2=80=94=20brand=20importer=20hardening?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - V-B06 enum validation: the five VALID_* sets (VALID_BRAND_CATEGORIES, VALID_VALUE_TRAJECTORIES, VALID_SCARCITY_CLASSES, VALID_BRAND_TIERS, VALID_CURRENCY_DENOMINATIONS) were defined but never referenced. brand_products.brand_category etc. are plain TEXT with no CHECK constraints, so a typo like `brand_category = "terrior"` silently imported. `validate_brands` now runs a V-B06 pass that asserts every enum column is a member of its VALID_* set. V-B01..V-B05 + V-B06 all reported together on import failure. - Explicit transaction wrapper: the clear-then-reimport cycle (10 DELETEs followed by 9 imports and structural validation) used to depend on Python's implicit-deferred-transaction semantics and sys.exit() on validation failure. A crash mid-import could leave the DB with some tables empty and others intact. The body now runs inside `conn.execute("BEGIN")` + try/except with an explicit `_ImportAborted` for validation failures and a `BaseException` catch-all for KeyboardInterrupt / programmer errors. All failure paths rollback before exit; the commit only fires after structural validation passes. Dry-run leaves the transaction open so the coverage check below can still SELECT against in-memory state. - system_fiscal docstring: previously cited the D-189 §6 derived formula (`collection_efficiency = 1.0 - shadow_economy_intensity × 0.6`) while the implementation hardcodes `collection_efficiency = 0.85` for every system. The docstring now explicitly states these are Phase 2 placeholder values (with named constants PHASE2_CORP_TAX_RATE and PHASE2_COLLECTION_EFFICIENCY) and calls out the shadow_economy.toml pipeline as the Phase 3 follow-up. --- tooling/economy-db/import_economics.py | 262 +++++++++++++++---------- 1 file changed, 160 insertions(+), 102 deletions(-) diff --git a/tooling/economy-db/import_economics.py b/tooling/economy-db/import_economics.py index 2ef3d697c..67d4d58a7 100755 --- a/tooling/economy-db/import_economics.py +++ b/tooling/economy-db/import_economics.py @@ -42,6 +42,12 @@ CORPORATIONS_DIR = REPO_ROOT / "wiki" / "corporations" BRANDS_TOML = REPO_ROOT / "wiki" / "economics" / "corporations" / "brands.toml" +class _ImportAborted(Exception): + """Raised internally by main() when a validation step wants a clean + rollback + exit 1. Caught only by main(); error messages are printed + before raising so the user sees them.""" + + # --------------------------------------------------------------------------- # Schema migration — add new tables and columns to existing DB # --------------------------------------------------------------------------- @@ -805,11 +811,18 @@ def import_brands( def import_system_fiscal(conn: sqlite3.Connection, dry_run: bool) -> int: - """Populate system_fiscal from star_systems shadow_economy data. + """Populate system_fiscal with hardcoded Phase 2 values. - collection_efficiency = 1.0 - shadow_economy_intensity × 0.6 (D-189 section 6). - shadow_economy_intensity comes from the shadow_economy.toml pipeline or defaults to 0. - Uses corp_tax_rate = 0.22 (default) for all systems in Phase 2. + Phase 2 values (NOT derived from D-189 §6 yet): + - corp_tax_rate = 0.22 (flat default) + - collection_efficiency = 0.85 (mid-reach average placeholder) + + The D-189 §6 formula `collection_efficiency = 1.0 - shadow_economy_intensity × 0.6` + is deliberately NOT implemented here — `shadow_economy_intensity` is not + yet per-system in the DB (pending the shadow_economy.toml pipeline). When + that pipeline lands, replace the hardcoded 0.85 with the derivation and + wire `shadow_economy_intensity` through the SELECT. Tracked as a Phase 3 + follow-up. """ inhabited = conn.execute(""" SELECT ss.system_id, COALESCE(se.population, 0) @@ -819,12 +832,13 @@ def import_system_fiscal(conn: sqlite3.Connection, dry_run: bool) -> int: ORDER BY ss.system_id """).fetchall() - rows = [] - for system_id, _pop in inhabited: - # Phase 2: shadow_economy_intensity not yet per-system in DB. - # Default collection_efficiency to 0.85 (mid-reach average) until - # shadow_economy.toml pipeline is wired (Phase 3). - rows.append((system_id, 0.22, 0.85)) + PHASE2_CORP_TAX_RATE = 0.22 + PHASE2_COLLECTION_EFFICIENCY = 0.85 + + rows = [ + (system_id, PHASE2_CORP_TAX_RATE, PHASE2_COLLECTION_EFFICIENCY) + for system_id, _pop in inhabited + ] if not dry_run: conn.executemany( @@ -837,13 +851,15 @@ def import_system_fiscal(conn: sqlite3.Connection, dry_run: bool) -> int: def validate_brands(conn: sqlite3.Connection) -> list[str]: - """Brand layer structural validation rules V-B01 through V-B05. + """Brand layer structural validation rules V-B01 through V-B06. V-B01: Every brand_products row has a valid corp_id (FK to corporations). V-B02: Every brand_inputs row has valid brand_product_id and commodity_id FKs. V-B03: Every halo brand has at least one brand_inputs entry (demand stub must consume). V-B04: Every volume tier must reference an existing halo brand_product_id. V-B05: No brand_product_id is used as halo_brand_id by a non-volume-tier product. + V-B06: Every enum column (brand_category, value_trajectory, scarcity_class, + brand_tier, currency_denomination) is a member of its VALID_* set. """ errors: list[str] = [] @@ -922,6 +938,27 @@ def validate_brands(conn: sqlite3.Connection) -> list[str]: f"which has brand_tier='{parent_tier}', not 'halo'" ) + # V-B06: every enum column is in its VALID_* set. The SQL columns are + # plain TEXT without CHECK constraints, so a typo like `terrior` would + # otherwise silently import. + enum_checks = [ + ("brand_category", VALID_BRAND_CATEGORIES), + ("value_trajectory", VALID_VALUE_TRAJECTORIES), + ("scarcity_class", VALID_SCARCITY_CLASSES), + ("brand_tier", VALID_BRAND_TIERS), + ("currency_denomination", VALID_CURRENCY_DENOMINATIONS), + ] + for column, valid_set in enum_checks: + bad = conn.execute( + f"SELECT brand_product_id, {column} FROM brand_products" + ).fetchall() + for pid, value in bad: + if value not in valid_set: + errors.append( + f"V-B06: brand_product '{pid}' has {column}='{value}' — " + f"must be one of {sorted(valid_set)}" + ) + return errors @@ -954,107 +991,128 @@ def main(): conn = sqlite3.connect(str(db_path)) conn.execute("PRAGMA foreign_keys=ON") - # 1. Migrate schema - print(" [1/10] Schema migration...") - for table, col, col_type in COLUMN_MIGRATIONS: - _add_column(conn, table, col, col_type) - conn.executescript(MIGRATION_SQL) - print(" tables and columns ready") + # The clear-then-reimport cycle below runs as a single explicit + # transaction. Any crash, validation error, or KeyboardInterrupt + # between the first DELETE and the final commit rolls everything + # back — the DB never ends up half-cleared with stale rows in some + # tables and empty rows in others. On success we commit exactly + # once, immediately after structural validation passes. + conn.execute("BEGIN") + try: + # 1. Migrate schema (idempotent, inside the tx so a crash here + # leaves no half-applied ALTER TABLE.) + print(" [1/10] Schema migration...") + for table, col, col_type in COLUMN_MIGRATIONS: + _add_column(conn, table, col, col_type) + conn.executescript(MIGRATION_SQL) + print(" tables and columns ready") - # Clear economics tables in FK-safe order (children before parents) - # corp_presence cleared here; corporations table is append-only (never cleared) - if not args.dry_run: - conn.execute("DELETE FROM corp_presence") - conn.execute("DELETE FROM brand_inputs") - conn.execute("DELETE FROM brand_products") - conn.execute("DELETE FROM system_fiscal") - conn.execute("DELETE FROM corp_financial_state") - conn.execute("DELETE FROM corp_lifecycle_events") - conn.execute("DELETE FROM chain_inputs") - conn.execute("DELETE FROM production_chains") - conn.execute("DELETE FROM commodities") - conn.execute("DELETE FROM gate_links") + # Clear economics tables in FK-safe order (children before parents) + # corp_presence cleared here; corporations table is append-only + # (never cleared). + if not args.dry_run: + conn.execute("DELETE FROM corp_presence") + conn.execute("DELETE FROM brand_inputs") + conn.execute("DELETE FROM brand_products") + conn.execute("DELETE FROM system_fiscal") + conn.execute("DELETE FROM corp_financial_state") + conn.execute("DELETE FROM corp_lifecycle_events") + conn.execute("DELETE FROM chain_inputs") + conn.execute("DELETE FROM production_chains") + conn.execute("DELETE FROM commodities") + conn.execute("DELETE FROM gate_links") - # 2. Gate links - print(" [2/10] Importing gate links...") - n_links = import_gate_links(conn, args.dry_run) - print(f" {n_links} rows (bidirectional)") + # 2. Gate links + print(" [2/10] Importing gate links...") + n_links = import_gate_links(conn, args.dry_run) + print(f" {n_links} rows (bidirectional)") - # 3. Commodities - print(" [3/10] Importing commodities...") - n_commodities = import_commodities(conn, args.dry_run) - print(f" {n_commodities} commodities") + # 3. Commodities + print(" [3/10] Importing commodities...") + n_commodities = import_commodities(conn, args.dry_run) + print(f" {n_commodities} commodities") - # 4. Production chains - print(" [4/10] Importing production chains...") - n_chains, n_inputs = import_chains(conn, args.dry_run) - print(f" {n_chains} chains, {n_inputs} inputs") + # 4. Production chains + print(" [4/10] Importing production chains...") + n_chains, n_inputs = import_chains(conn, args.dry_run) + print(f" {n_chains} chains, {n_inputs} inputs") - # 5. Currency zones - print(" [5/10] Setting currency zones...") - zones = set_currency_zones(conn, args.dry_run) - for zone, count in sorted(zones.items()): - print(f" {zone}: {count}") + # 5. Currency zones + print(" [5/10] Setting currency zones...") + zones = set_currency_zones(conn, args.dry_run) + for zone, count in sorted(zones.items()): + print(f" {zone}: {count}") - # 6. Gate energy connectivity (D-186) — must run after currency zones - print(" [6/10] Setting gate energy connectivity...") - energy = set_gate_energy(conn, args.dry_run) - for label, count in sorted(energy.items()): - print(f" {label}: {count}") + # 6. Gate energy connectivity (D-186) — must run after currency zones + print(" [6/10] Setting gate energy connectivity...") + energy = set_gate_energy(conn, args.dry_run) + for label, count in sorted(energy.items()): + print(f" {label}: {count}") - # 7. Sync corporations from wiki (D-182: hard error on name divergence) - print(" [7/10] Syncing corporations...") - corp_errors = sync_corporations(conn, wiki_corps, args.dry_run) - if corp_errors: - print(f" ERRORS ({len(corp_errors)}) — name divergence detected (D-182):") - for e in corp_errors: - print(f" - {e}") - print(" Fix: update wiki title or DB proper_name to match, then re-run.") + # 7. Sync corporations from wiki (D-182: hard error on name divergence) + print(" [7/10] Syncing corporations...") + corp_errors = sync_corporations(conn, wiki_corps, args.dry_run) + if corp_errors: + print(f" ERRORS ({len(corp_errors)}) — name divergence detected (D-182):") + for e in corp_errors: + print(f" - {e}") + print(" Fix: update wiki title or DB proper_name to match, then re-run.") + raise _ImportAborted() + n_db_corps = conn.execute("SELECT COUNT(*) FROM corporations").fetchone()[0] + print(f" {n_db_corps} corporations in DB ({len(wiki_corps)} from wiki)") + + # 8. Corp presence from wiki headquarters data + print(" [8/10] Importing corp presence...") + commodity_ids = { + r[0] for r in conn.execute("SELECT commodity_id FROM commodities").fetchall() + } + n_presence = import_corp_presence(conn, wiki_corps, commodity_ids, args.dry_run) + print(f" {n_presence} corp_presence rows") + + # 9. Brand products and inputs (D-189, #827) + print(" [9/10] Importing brand products and inputs...") + n_brands, n_brand_inputs = import_brands(conn, args.dry_run) + print(f" {n_brands} brand_products, {n_brand_inputs} brand_inputs") + + # 10. System fiscal parameters (D-189 section 6) + print(" [10/10] Populating system_fiscal...") + n_fiscal = import_system_fiscal(conn, args.dry_run) + print(f" {n_fiscal} system_fiscal rows") + + # Validate structural integrity (FK, chain refs, chain completeness). + # These errors indicate broken imported data — do NOT commit. + print("\n Validating structural integrity...") + struct_errors = validate(conn) + struct_errors.extend(validate_brands(conn)) + if struct_errors: + print(f" STRUCTURAL ERRORS ({len(struct_errors)}) — rolling back:") + for e in struct_errors: + print(f" - {e}") + raise _ImportAborted() + print(" FK integrity, chain completeness, and brand layer (V-B01–V-B06) OK") + + # Commit all imported data (corps, presence, etc.) before coverage check. + # Coverage validation is a Phase 2 gate (D-175) — data should be persisted + # so tools can query it and report gaps clearly. + if not args.dry_run: + conn.commit() + print(" Data committed.") + else: + # Dry-run: leave the transaction open so the coverage check below + # can still SELECT against the in-memory imported data. The + # transaction is discarded when conn.close() runs on exit. + print(" Dry run — no changes written.") + except _ImportAborted: + conn.rollback() conn.close() sys.exit(1) - n_db_corps = conn.execute("SELECT COUNT(*) FROM corporations").fetchone()[0] - print(f" {n_db_corps} corporations in DB ({len(wiki_corps)} from wiki)") - - # 8. Corp presence from wiki headquarters data - print(" [8/10] Importing corp presence...") - commodity_ids = { - r[0] for r in conn.execute("SELECT commodity_id FROM commodities").fetchall() - } - n_presence = import_corp_presence(conn, wiki_corps, commodity_ids, args.dry_run) - print(f" {n_presence} corp_presence rows") - - # 9. Brand products and inputs (D-189, #827) - print(" [9/10] Importing brand products and inputs...") - n_brands, n_brand_inputs = import_brands(conn, args.dry_run) - print(f" {n_brands} brand_products, {n_brand_inputs} brand_inputs") - - # 10. System fiscal parameters (D-189 section 6) - print(" [10/10] Populating system_fiscal...") - n_fiscal = import_system_fiscal(conn, args.dry_run) - print(f" {n_fiscal} system_fiscal rows") - - # Validate structural integrity (FK, chain refs, chain completeness). - # These errors indicate broken imported data — do NOT commit. - print("\n Validating structural integrity...") - struct_errors = validate(conn) - struct_errors.extend(validate_brands(conn)) - if struct_errors: - print(f" STRUCTURAL ERRORS ({len(struct_errors)}) — rolling back:") - for e in struct_errors: - print(f" - {e}") + except BaseException: + # Any other exception (KeyboardInterrupt, MemoryError, DB error, + # programmer error) triggers a rollback so the DB is never left in + # a half-imported state. Re-raise so the user sees the traceback. + conn.rollback() conn.close() - sys.exit(1) - else: - print(" FK integrity, chain completeness, and brand layer (V-B01–V-B05) OK") - - # Commit all imported data (corps, presence, etc.) before coverage check. - # Coverage validation is a Phase 2 gate (D-175) — data should be persisted - # so tools can query it and report gaps clearly. - if not args.dry_run: - conn.commit() - print(" Data committed.") - else: - print(" Dry run — no changes written.") + raise # Validate coverage (hard errors per D-175, but after commit so data is usable). print("\n Validating coverage (D-175 Phase 2 gate)...")