pre-review fixes
This commit is contained in:
@@ -46,6 +46,7 @@ tests/ # pytest, uses seeded_repo fixture from conftest.py
|
|||||||
|
|
||||||
## Key patterns
|
## 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`.
|
- **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 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`.
|
- **`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`.
|
||||||
|
|||||||
37
LOGIC DOCS/002-guardrails-via-intrinsic-properties.md
Normal file
37
LOGIC DOCS/002-guardrails-via-intrinsic-properties.md
Normal file
@@ -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.
|
||||||
@@ -227,19 +227,17 @@ def review(ctx, combination_id):
|
|||||||
|
|
||||||
@main.command()
|
@main.command()
|
||||||
@click.argument("domain_name")
|
@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("--top", "-n", default=20, type=int, help="Number of results to export")
|
||||||
@click.option("--output", "-o", default=None, help="Output file path")
|
@click.option("--output", "-o", default=None, help="Output file path")
|
||||||
@click.pass_context
|
@click.pass_context
|
||||||
def export(ctx, domain_name, fmt, top, output):
|
def export(ctx, domain_name, top, output):
|
||||||
"""Export results to a report."""
|
"""Export results to a Markdown report."""
|
||||||
repo = _get_repo(ctx.obj["db"])
|
repo = _get_repo(ctx.obj["db"])
|
||||||
top_results = repo.get_top_results(domain_name, limit=top)
|
top_results = repo.get_top_results(domain_name, limit=top)
|
||||||
if not top_results:
|
if not top_results:
|
||||||
click.echo(f"No results for domain '{domain_name}'.")
|
click.echo(f"No results for domain '{domain_name}'.")
|
||||||
return
|
return
|
||||||
|
|
||||||
if fmt == "md":
|
|
||||||
lines = [f"# {domain_name} — Top {len(top_results)} Concepts\n"]
|
lines = [f"# {domain_name} — Top {len(top_results)} Concepts\n"]
|
||||||
for i, r in enumerate(top_results, 1):
|
for i, r in enumerate(top_results, 1):
|
||||||
combo = r["combination"]
|
combo = r["combination"]
|
||||||
@@ -259,8 +257,6 @@ def export(ctx, domain_name, fmt, top, output):
|
|||||||
click.echo(f"Exported to {output}")
|
click.echo(f"Exported to {output}")
|
||||||
else:
|
else:
|
||||||
click.echo(content)
|
click.echo(content)
|
||||||
else:
|
|
||||||
click.echo(f"Unsupported format: {fmt}")
|
|
||||||
|
|
||||||
|
|
||||||
@main.group()
|
@main.group()
|
||||||
|
|||||||
@@ -268,8 +268,8 @@ class Repository:
|
|||||||
by_key.setdefault(r["key"], []).append(r["value"])
|
by_key.setdefault(r["key"], []).append(r["value"])
|
||||||
return [DomainConstraint(key=k, allowed_values=v) for k, v in by_key.items()]
|
return [DomainConstraint(key=k, allowed_values=v) for k, v in by_key.items()]
|
||||||
|
|
||||||
def get_domain(self, name: str) -> Domain | None:
|
def _load_domain(self, where: str, param: str | int) -> Domain | None:
|
||||||
row = self.conn.execute("SELECT * FROM domains WHERE name = ?", (name,)).fetchone()
|
row = self.conn.execute(f"SELECT * FROM domains WHERE {where} = ?", (param,)).fetchone()
|
||||||
if not row:
|
if not row:
|
||||||
return None
|
return None
|
||||||
weights = self.conn.execute(
|
weights = self.conn.execute(
|
||||||
@@ -296,37 +296,15 @@ class Repository:
|
|||||||
constraints=self._load_domain_constraints(row["id"]),
|
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]:
|
def list_domains(self) -> list[Domain]:
|
||||||
rows = self.conn.execute("SELECT name FROM domains ORDER BY name").fetchall()
|
rows = self.conn.execute("SELECT name FROM domains ORDER BY name").fetchall()
|
||||||
return [self.get_domain(r["name"]) for r in rows]
|
return [self.get_domain(r["name"]) for r in rows]
|
||||||
|
|
||||||
def get_domain_by_id(self, domain_id: int) -> Domain | None:
|
def get_domain_by_id(self, domain_id: int) -> Domain | None:
|
||||||
row = self.conn.execute("SELECT * FROM domains WHERE id = ?", (domain_id,)).fetchone()
|
return self._load_domain("id", domain_id)
|
||||||
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"]),
|
|
||||||
)
|
|
||||||
|
|
||||||
def update_domain(self, domain_id: int, name: str, description: str) -> None:
|
def update_domain(self, domain_id: int, name: str, description: str) -> None:
|
||||||
self.conn.execute(
|
self.conn.execute(
|
||||||
|
|||||||
@@ -338,7 +338,7 @@ RENEWABLE_ACTUATORS: list[Entity] = [
|
|||||||
Dependency("energy", "energy_form", "radiation_pressure", None, "requires"),
|
Dependency("energy", "energy_form", "radiation_pressure", None, "requires"),
|
||||||
Dependency("environment", "atmosphere", "vacuum_or_thin", None, "requires"),
|
Dependency("environment", "atmosphere", "vacuum_or_thin", None, "requires"),
|
||||||
Dependency("environment", "star_proximity", "true", 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", "thrust_profile", "continuous_low", None, "provides"),
|
||||||
Dependency("force", "power_density", "0.01", "W/kg", "provides"),
|
Dependency("force", "power_density", "0.01", "W/kg", "provides"),
|
||||||
Dependency("environment", "medium", "space", None, "requires"),
|
Dependency("environment", "medium", "space", None, "requires"),
|
||||||
@@ -393,6 +393,7 @@ ROCKET_ACTUATORS: list[Entity] = [
|
|||||||
dependencies=[
|
dependencies=[
|
||||||
Dependency("energy", "energy_form", "nuclear_thermal", None, "requires"),
|
Dependency("energy", "energy_form", "nuclear_thermal", None, "requires"),
|
||||||
Dependency("physical", "mass", "1500", "kg", "range_min"),
|
Dependency("physical", "mass", "1500", "kg", "range_min"),
|
||||||
|
Dependency("physical", "footprint", "20", "m²", "range_min"),
|
||||||
Dependency("force", "thrust_profile", "extreme_continuous", None, "provides"),
|
Dependency("force", "thrust_profile", "extreme_continuous", None, "provides"),
|
||||||
Dependency("force", "power_density", "50", "W/kg", "provides"),
|
Dependency("force", "power_density", "50", "W/kg", "provides"),
|
||||||
Dependency("material", "radiation_shielding", "true", None, "requires"),
|
Dependency("material", "radiation_shielding", "true", None, "requires"),
|
||||||
@@ -644,6 +645,7 @@ PROPELLANT_STORAGE: list[Entity] = [
|
|||||||
Dependency("energy", "energy_form", "nuclear_thermal", None, "provides"),
|
Dependency("energy", "energy_form", "nuclear_thermal", None, "provides"),
|
||||||
Dependency("infrastructure", "fuel_infrastructure", "nuclear_fuel", None, "requires"),
|
Dependency("infrastructure", "fuel_infrastructure", "nuclear_fuel", None, "requires"),
|
||||||
Dependency("physical", "mass", "500", "kg", "range_min"),
|
Dependency("physical", "mass", "500", "kg", "range_min"),
|
||||||
|
Dependency("physical", "footprint", "5", "m²", "range_min"),
|
||||||
Dependency("physical", "energy_density", "1800000000", "J/kg", "provides"),
|
Dependency("physical", "energy_density", "1800000000", "J/kg", "provides"),
|
||||||
Dependency("material", "radiation_shielding", "true", None, "requires"),
|
Dependency("material", "radiation_shielding", "true", None, "requires"),
|
||||||
],
|
],
|
||||||
|
|||||||
@@ -2,7 +2,6 @@
|
|||||||
|
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
import math
|
|
||||||
import os
|
import os
|
||||||
import secrets
|
import secrets
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
@@ -55,53 +54,10 @@ def close_db(exc: BaseException | None = None) -> None:
|
|||||||
repo.conn.close()
|
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:
|
def create_app() -> Flask:
|
||||||
app = Flask(__name__)
|
app = Flask(__name__)
|
||||||
app.secret_key = _load_or_generate_secret_key()
|
app.secret_key = _load_or_generate_secret_key()
|
||||||
|
|
||||||
app.jinja_env.filters["si"] = _si_format
|
|
||||||
app.jinja_env.filters["qty"] = format_quantity
|
app.jinja_env.filters["qty"] = format_quantity
|
||||||
|
|
||||||
app.teardown_appcontext(close_db)
|
app.teardown_appcontext(close_db)
|
||||||
|
|||||||
@@ -59,12 +59,6 @@ def entity_detail(entity_id: int):
|
|||||||
return render_template("entities/detail.html", entity=entity)
|
return render_template("entities/detail.html", entity=entity)
|
||||||
|
|
||||||
|
|
||||||
@bp.route("/<int:entity_id>/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("/<int:entity_id>/delete", methods=["POST"])
|
@bp.route("/<int:entity_id>/delete", methods=["POST"])
|
||||||
def entity_delete(entity_id: int):
|
def entity_delete(entity_id: int):
|
||||||
repo = get_repo()
|
repo = get_repo()
|
||||||
|
|||||||
@@ -31,7 +31,7 @@
|
|||||||
<td>{{ e.description }}</td>
|
<td>{{ e.description }}</td>
|
||||||
<td>{{ e.dependencies|length }}</td>
|
<td>{{ e.dependencies|length }}</td>
|
||||||
<td>
|
<td>
|
||||||
<a href="{{ url_for('entities.entity_edit', entity_id=e.id) }}" class="btn btn-sm">Edit</a>
|
<a href="{{ url_for('entities.entity_detail', entity_id=e.id) }}" class="btn btn-sm">Edit</a>
|
||||||
</td>
|
</td>
|
||||||
</tr>
|
</tr>
|
||||||
{% endfor %}
|
{% endfor %}
|
||||||
|
|||||||
Reference in New Issue
Block a user