From 63295ab80e6e0e35d53fa7171004fada140f6d52 Mon Sep 17 00:00:00 2001 From: Andrew Simonson Date: Sat, 25 Jul 2026 16:42:39 -0500 Subject: [PATCH] pre-review fixes --- CLAUDE.md | 1 + ...002-guardrails-via-intrinsic-properties.md | 37 ++++++++++++++++ src/physcom/cli.py | 42 ++++++++---------- src/physcom/db/repository.py | 34 +++----------- src/physcom/seed/transport_example.py | 4 +- src/physcom_web/app.py | 44 ------------------- src/physcom_web/routes/entities.py | 6 --- src/physcom_web/templates/entities/list.html | 2 +- 8 files changed, 67 insertions(+), 103 deletions(-) create mode 100644 LOGIC DOCS/002-guardrails-via-intrinsic-properties.md diff --git a/CLAUDE.md b/CLAUDE.md index 0239bde..65f071b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -46,6 +46,7 @@ tests/ # pytest, uses seeded_repo fixture from conftest.py ## Key patterns +- **Guardrails are emergent, not authored.** Entities never reference each other by name. Each declares only its own intrinsic physical envelope (`requires`/`provides`/`excludes` for categorical fit, `range_min`/`range_max` for numeric floors/ceilings on shared keys like `mass`, `footprint`, `energy_density`). `ConstraintResolver` cross-checks these generically across every pair in a combo — a nuclear reactor is blocked from a bicycle because `mass range_min` collides with `mass range_max`, not because any code says "nuclear + bicycle = blocked." When adding physical realism to an entity, add a property to the entity — never a pairwise special case in `ConstraintResolver`. A category missing a floor it should have is a silent guardrail hole (blocks nothing, looks fine). See `LOGIC DOCS/002-guardrails-via-intrinsic-properties.md`. - **Repository is the only DB interface.** No raw SQL outside `repository.py`. - **Pipeline is combo-first**: each combo goes through all requested passes before the next combo starts. Progress is persisted per-combo (crash-safe, resumable). - **`pipeline_runs` table** tracks run lifecycle: pending → running → completed/failed/cancelled. The web route creates the record, then starts a background thread with its own `sqlite3.Connection`. diff --git a/LOGIC DOCS/002-guardrails-via-intrinsic-properties.md b/LOGIC DOCS/002-guardrails-via-intrinsic-properties.md new file mode 100644 index 0000000..e6273c9 --- /dev/null +++ b/LOGIC DOCS/002-guardrails-via-intrinsic-properties.md @@ -0,0 +1,37 @@ +# Guardrails via intrinsic properties, not special cases + +## The goal + +Full recombination across the existing catalog — any platform × any actuator × any energy source — should stay possible, while physically impossible pairings (a nuclear reactor on a bicycle, a solar sail in an atmosphere) get blocked automatically. Neither half is negotiable: too loose and the results are nonsense, too special-cased and the combinatorics stop being the point of the project. + +## The mechanism (already in place) + +Entities never reference each other. Each one only declares its own physical envelope: + +- `requires`/`provides`/`excludes` — categorical compatibility (`energy_form`, `medium`, `atmosphere`) +- `range_min`/`range_max` — numeric floors and ceilings on a shared key (`mass`, `footprint`, `energy_density`, ...) + +`ConstraintResolver` (`src/physcom/engine/constraint_resolver.py`) then runs generic, entity-agnostic rules across every pair in a combo: requires-vs-excludes, mutual exclusion, range incompatibility, provides-vs-range deficit, unmet requirements. A combo gets blocked because two numbers or two category tags collided — never because code somewhere says "if X and Y, block." Blocking is emergent from the data, not authored per pair. + +This is what makes "nuclear reactor won't fit on a bicycle" work today: Nuclear Thermal Drive declares `mass range_min = 1500kg`, Light Personal Vehicle declares `mass range_max = 60kg`, and Rule 3 (`_check_range_incompatibility`) blocks the combo without either entity knowing the other exists. + +## Why this breaks silently + +The mechanism only blocks what the data describes. A category missing a `mass range_min` or `footprint range_min` it should physically have doesn't get blocked — not because the rule is wrong, but because nothing told the resolver there was a limit. This is an easy failure mode: the constraint system looks like it's working (it blocks the pairs someone thought to test) while quietly admitting nonsense pairs nobody happened to check. + +Two such gaps were found and fixed 2026-07 (see git history around this doc's commit): + +- Solar Sail declared its size under the key `surface_area` instead of `footprint` — a naming mismatch made it invisible to every cross-check, silently a no-op. +- Nuclear Thermal Drive and Nuclear Fuel had a `mass range_min` but no `footprint range_min` — reactor mass was guarded, reactor size was not. + +Both were fixed by adding/renaming a `Dependency`, zero changes to `ConstraintResolver` itself. That's the intended shape of a fix here. + +## The standing question for review + +For every actuator and energy_storage entity: what is the **full set of intrinsic physical floors and ceilings** a real version of this technology imposes at any scale — structural mass, footprint, radiation shielding, minimum coolant/containment volume, thermal rejection surface area, etc. — and is each one expressed as a `range_min`/`range_max`/`provides`/`requires` dependency using a key that **already exists** in the vocabulary (`grep -oP 'Dependency\("[a-z]+", "\K[a-z_]+'` over `seed/transport_example.py` lists it), so it actually cross-checks against every other entity that shares that key? + +A missing floor is a silent guardrail hole, not a missing detail. + +## Non-goal + +Do not add pairwise special cases (`if actuator == "Nuclear Thermal Drive" and platform == "..."`). That defeats the architecture — the whole point is that the catalog recombines freely and physics falls out of shared, generic constraints. Any new realism belongs on the entity as a property, not in `ConstraintResolver` as a rule. diff --git a/src/physcom/cli.py b/src/physcom/cli.py index 83708fb..c9a0da4 100644 --- a/src/physcom/cli.py +++ b/src/physcom/cli.py @@ -227,40 +227,36 @@ def review(ctx, combination_id): @main.command() @click.argument("domain_name") -@click.option("--format", "fmt", default="md", help="Export format (md)") @click.option("--top", "-n", default=20, type=int, help="Number of results to export") @click.option("--output", "-o", default=None, help="Output file path") @click.pass_context -def export(ctx, domain_name, fmt, top, output): - """Export results to a report.""" +def export(ctx, domain_name, top, output): + """Export results to a Markdown report.""" repo = _get_repo(ctx.obj["db"]) top_results = repo.get_top_results(domain_name, limit=top) if not top_results: click.echo(f"No results for domain '{domain_name}'.") return - if fmt == "md": - lines = [f"# {domain_name} — Top {len(top_results)} Concepts\n"] - for i, r in enumerate(top_results, 1): - combo = r["combination"] - entity_names = " + ".join(e.name for e in combo.entities) - lines.append(f"## {i}. {entity_names} (score: {r['composite_score']:.4f})") - if r["novelty_flag"]: - lines.append(f"**Novelty:** {r['novelty_flag']}") - if r["llm_review"]: - lines.append(f"\n{r['llm_review']}") - if r["human_notes"]: - lines.append(f"\n*Notes:* {r['human_notes']}") - lines.append("") + lines = [f"# {domain_name} — Top {len(top_results)} Concepts\n"] + for i, r in enumerate(top_results, 1): + combo = r["combination"] + entity_names = " + ".join(e.name for e in combo.entities) + lines.append(f"## {i}. {entity_names} (score: {r['composite_score']:.4f})") + if r["novelty_flag"]: + lines.append(f"**Novelty:** {r['novelty_flag']}") + if r["llm_review"]: + lines.append(f"\n{r['llm_review']}") + if r["human_notes"]: + lines.append(f"\n*Notes:* {r['human_notes']}") + lines.append("") - content = "\n".join(lines) - if output: - Path(output).write_text(content) - click.echo(f"Exported to {output}") - else: - click.echo(content) + content = "\n".join(lines) + if output: + Path(output).write_text(content) + click.echo(f"Exported to {output}") else: - click.echo(f"Unsupported format: {fmt}") + click.echo(content) @main.group() diff --git a/src/physcom/db/repository.py b/src/physcom/db/repository.py index acbcb9a..1d2d6db 100644 --- a/src/physcom/db/repository.py +++ b/src/physcom/db/repository.py @@ -268,8 +268,8 @@ class Repository: by_key.setdefault(r["key"], []).append(r["value"]) return [DomainConstraint(key=k, allowed_values=v) for k, v in by_key.items()] - def get_domain(self, name: str) -> Domain | None: - row = self.conn.execute("SELECT * FROM domains WHERE name = ?", (name,)).fetchone() + def _load_domain(self, where: str, param: str | int) -> Domain | None: + row = self.conn.execute(f"SELECT * FROM domains WHERE {where} = ?", (param,)).fetchone() if not row: return None weights = self.conn.execute( @@ -296,37 +296,15 @@ class Repository: constraints=self._load_domain_constraints(row["id"]), ) + def get_domain(self, name: str) -> Domain | None: + return self._load_domain("name", name) + def list_domains(self) -> list[Domain]: rows = self.conn.execute("SELECT name FROM domains ORDER BY name").fetchall() return [self.get_domain(r["name"]) for r in rows] def get_domain_by_id(self, domain_id: int) -> Domain | None: - row = self.conn.execute("SELECT * FROM domains WHERE id = ?", (domain_id,)).fetchone() - if not row: - return None - weights = self.conn.execute( - """SELECT m.name, m.unit, dmw.weight, dmw.norm_min, dmw.norm_max, - dmw.metric_id, dmw.lower_is_better - FROM domain_metric_weights dmw - JOIN metrics m ON dmw.metric_id = m.id - WHERE dmw.domain_id = ?""", - (row["id"],), - ).fetchall() - return Domain( - id=row["id"], - name=row["name"], - description=row["description"] or "", - metric_bounds=[ - MetricBound( - metric_name=w["name"], weight=w["weight"], - norm_min=w["norm_min"], norm_max=w["norm_max"], - metric_id=w["metric_id"], unit=w["unit"] or "", - lower_is_better=bool(w["lower_is_better"]), - ) - for w in weights - ], - constraints=self._load_domain_constraints(row["id"]), - ) + return self._load_domain("id", domain_id) def update_domain(self, domain_id: int, name: str, description: str) -> None: self.conn.execute( diff --git a/src/physcom/seed/transport_example.py b/src/physcom/seed/transport_example.py index a36fb6d..58d33c7 100644 --- a/src/physcom/seed/transport_example.py +++ b/src/physcom/seed/transport_example.py @@ -338,7 +338,7 @@ RENEWABLE_ACTUATORS: list[Entity] = [ Dependency("energy", "energy_form", "radiation_pressure", None, "requires"), Dependency("environment", "atmosphere", "vacuum_or_thin", None, "requires"), Dependency("environment", "star_proximity", "true", None, "requires"), - Dependency("physical", "surface_area", "100", "m²", "range_min"), + Dependency("physical", "footprint", "100", "m²", "range_min"), Dependency("force", "thrust_profile", "continuous_low", None, "provides"), Dependency("force", "power_density", "0.01", "W/kg", "provides"), Dependency("environment", "medium", "space", None, "requires"), @@ -393,6 +393,7 @@ ROCKET_ACTUATORS: list[Entity] = [ dependencies=[ Dependency("energy", "energy_form", "nuclear_thermal", None, "requires"), Dependency("physical", "mass", "1500", "kg", "range_min"), + Dependency("physical", "footprint", "20", "m²", "range_min"), Dependency("force", "thrust_profile", "extreme_continuous", None, "provides"), Dependency("force", "power_density", "50", "W/kg", "provides"), Dependency("material", "radiation_shielding", "true", None, "requires"), @@ -644,6 +645,7 @@ PROPELLANT_STORAGE: list[Entity] = [ Dependency("energy", "energy_form", "nuclear_thermal", None, "provides"), Dependency("infrastructure", "fuel_infrastructure", "nuclear_fuel", None, "requires"), Dependency("physical", "mass", "500", "kg", "range_min"), + Dependency("physical", "footprint", "5", "m²", "range_min"), Dependency("physical", "energy_density", "1800000000", "J/kg", "provides"), Dependency("material", "radiation_shielding", "true", None, "requires"), ], diff --git a/src/physcom_web/app.py b/src/physcom_web/app.py index fa51182..e148d4b 100644 --- a/src/physcom_web/app.py +++ b/src/physcom_web/app.py @@ -2,7 +2,6 @@ from __future__ import annotations -import math import os import secrets from pathlib import Path @@ -55,53 +54,10 @@ def close_db(exc: BaseException | None = None) -> None: repo.conn.close() -_SI_PREFIXES = [ - (1e12, "T"), - (1e9, "G"), - (1e6, "M"), - (1e3, "k"), -] - - -def _si_format(value: object) -> str: - """Format a number with SI prefixes for readability. - - Handles string inputs (like dep.value) by trying float conversion first. - Non-numeric values are returned as-is. - """ - if isinstance(value, str): - try: - num = float(value) - except (ValueError, TypeError): - return value - elif isinstance(value, (int, float)): - num = float(value) - else: - return str(value) - - if math.isnan(num) or math.isinf(num): - return str(value) - - abs_num = abs(num) - if abs_num < 1000: - # Small numbers: drop trailing zeros, cap at 4 significant figures - if num == int(num) and abs_num < 100: - return str(int(num)) - return f"{num:.4g}" - - for threshold, prefix in _SI_PREFIXES: - if abs_num >= threshold: - scaled = num / threshold - return f"{scaled:.4g}{prefix}" - - return f"{num:.4g}" - - def create_app() -> Flask: app = Flask(__name__) app.secret_key = _load_or_generate_secret_key() - app.jinja_env.filters["si"] = _si_format app.jinja_env.filters["qty"] = format_quantity app.teardown_appcontext(close_db) diff --git a/src/physcom_web/routes/entities.py b/src/physcom_web/routes/entities.py index 75d24cb..7bfcd0d 100644 --- a/src/physcom_web/routes/entities.py +++ b/src/physcom_web/routes/entities.py @@ -59,12 +59,6 @@ def entity_detail(entity_id: int): return render_template("entities/detail.html", entity=entity) -@bp.route("//edit") -def entity_edit(entity_id: int): - """Legacy route — redirect to detail page.""" - return redirect(url_for("entities.entity_detail", entity_id=entity_id)) - - @bp.route("//delete", methods=["POST"]) def entity_delete(entity_id: int): repo = get_repo() diff --git a/src/physcom_web/templates/entities/list.html b/src/physcom_web/templates/entities/list.html index 057fa8b..cd2ebeb 100644 --- a/src/physcom_web/templates/entities/list.html +++ b/src/physcom_web/templates/entities/list.html @@ -31,7 +31,7 @@ {{ e.description }} {{ e.dependencies|length }} - Edit + Edit {% endfor %}