diff --git a/src/physcom/db/repository.py b/src/physcom/db/repository.py index bdb10cf..35aeb40 100644 --- a/src/physcom/db/repository.py +++ b/src/physcom/db/repository.py @@ -20,6 +20,14 @@ class Repository: self.conn = conn self.conn.row_factory = sqlite3.Row + def commit(self) -> None: + """Explicit flush point, for callers batching writes with commit=False + below (see Pipeline.run: instant/deterministic passes defer commits + and flush in bulk, since a crash there just means cheap recompute; + LLM-call results still commit immediately, since those are slow/ + expensive to redo).""" + self.conn.commit() + # ── Dimensions ────────────────────────────────────────────── def ensure_dimension(self, name: str, description: str = "") -> int: @@ -422,7 +430,7 @@ class Repository: key = ",".join(str(eid) for eid in sorted(entity_ids)) return hashlib.sha256(key.encode()).hexdigest()[:16] - def save_combination(self, combination: Combination) -> Combination: + def save_combination(self, combination: Combination, commit: bool = True) -> Combination: entity_ids = [e.id for e in combination.entities] combination.hash = self.compute_hash(entity_ids) @@ -446,11 +454,12 @@ class Repository: "INSERT INTO combination_entities (combination_id, entity_id) VALUES (?, ?)", (combination.id, eid), ) - self.conn.commit() + if commit: + self.conn.commit() return combination def update_combination_status( - self, combo_id: int, status: str, block_reason: str | None = None + self, combo_id: int, status: str, block_reason: str | None = None, commit: bool = True ) -> None: # Don't downgrade from higher pass states — preserves human/LLM review data if status in ("scored", "llm_reviewed") or status.endswith("_fail"): @@ -470,7 +479,8 @@ class Repository: "UPDATE combinations SET status = ?, block_reason = ? WHERE id = ?", (status, block_reason, combo_id), ) - self.conn.commit() + if commit: + self.conn.commit() def get_combination(self, combo_id: int) -> Combination | None: row = self.conn.execute("SELECT * FROM combinations WHERE id = ?", (combo_id,)).fetchone() @@ -558,6 +568,7 @@ class Repository: combo_id: int, domain_id: int, scores: list[dict], + commit: bool = True, ) -> None: """Save per-metric scores. Each dict: metric_id, raw_value, normalized_score, estimation_method, confidence.""" for s in scores: @@ -569,7 +580,8 @@ class Repository: (combo_id, domain_id, s["metric_id"], s["raw_value"], s["normalized_score"], s["estimation_method"], s["confidence"]), ) - self.conn.commit() + if commit: + self.conn.commit() def save_result( self, @@ -581,6 +593,7 @@ class Repository: llm_review: str | None = None, human_notes: str | None = None, domain_block_reason: str | None = None, + commit: bool = True, ) -> None: self.conn.execute( """INSERT OR REPLACE INTO combination_results @@ -590,7 +603,8 @@ class Repository: (combo_id, domain_id, composite_score, novelty_flag, llm_review, human_notes, pass_reached, domain_block_reason), ) - self.conn.commit() + if commit: + self.conn.commit() def get_combination_scores(self, combo_id: int, domain_id: int) -> list[dict]: """Return per-metric scores for a combination in a domain.""" @@ -807,7 +821,7 @@ class Repository: return row["pass_reached"] if row else None def save_raw_estimates( - self, combo_id: int, domain_id: int, estimates: list[dict] + self, combo_id: int, domain_id: int, estimates: list[dict], commit: bool = True ) -> None: """Save raw metric estimates (pass 2) with normalized_score=NULL. @@ -822,7 +836,8 @@ class Repository: (combo_id, domain_id, e["metric_id"], e["raw_value"], e["estimation_method"], e["confidence"]), ) - self.conn.commit() + if commit: + self.conn.commit() def get_existing_result(self, combo_id: int, domain_id: int) -> dict | None: """Return the full combination_results row for resume logic.""" diff --git a/src/physcom/engine/pipeline.py b/src/physcom/engine/pipeline.py index f781f3d..d551b81 100644 --- a/src/physcom/engine/pipeline.py +++ b/src/physcom/engine/pipeline.py @@ -69,8 +69,12 @@ INFRASTRUCTURE_AVAILABILITY: dict[tuple[str, str], float] = { ("fuel_infrastructure", "xenon_propellant"): 0.05, } -# Crude freight-capacity proxy: kg of cargo per kg of vehicle structural mass. -CARGO_KG_PER_STRUCTURAL_KG: float = 500 +# Crude freight-capacity proxy: kg of cargo per kg of vehicle structural +# mass. Was 500 -- a magnitude error (500x cargo-to-structure has no real +# vehicle analog). Real cargo ships run deadweight/lightship ratios of +# roughly 1.5-4x depending on class; 2.5 is a reasonable general-cargo +# midpoint for this domain-agnostic proxy. +CARGO_KG_PER_STRUCTURAL_KG: float = 2.5 # How mechanically proven/predictable an energy form is in practice — distinct # from safety (risk when something goes wrong) and thrust_profile (delivery @@ -367,9 +371,13 @@ class Pipeline: combos = generate_combinations(self.repo, dimensions) result.total_generated = len(combos) - # Save all combinations to DB (also loads status for existing combos) + # Save all combinations to DB (also loads status for existing combos). + # Deferred commit -- registering combos is instant/deterministic, so a + # crash here just means re-running the (cheap) registration loop, not + # losing anything worth protecting with a commit per row. for combo in combos: - self.repo.save_combination(combo) + self.repo.save_combination(combo, commit=False) + self.repo.commit() if run_id is not None: self.repo.update_pipeline_run(run_id, total_combos=len(combos)) @@ -378,9 +386,20 @@ class Pipeline: bounds_by_name = {mb.metric_name: mb for mb in domain.metric_bounds} # ── Combo-first loop ───────────────────────────────────── + # Deterministic passes (1, 3, and 2 without an LLM) defer commits and + # get flushed periodically + in `finally` below -- a crash there costs + # a cheap recompute, not lost work worth committing per write. Pass 4 + # (and pass 2 with an LLM) commit immediately after each call: those + # are slow and crash-prone (see the QwQ timeout saga), so that result + # is worth protecting the moment it lands. + combos_since_commit = 0 try: for combo in combos: self._check_cancelled(run_id) + combos_since_commit += 1 + if combos_since_commit >= 200: + self.repo.commit() + combos_since_commit = 0 # Check existing progress for this combo in this domain existing_pass = self.repo.get_combo_pass_reached( @@ -399,7 +418,7 @@ class Pipeline: combo.status = "p1_fail" combo.block_reason = "; ".join(cr.violations) self.repo.update_combination_status( - combo.id, "p1_fail", combo.block_reason + combo.id, "p1_fail", combo.block_reason, commit=False ) # Save a result row so failed combos appear in results self.repo.save_result( @@ -407,13 +426,14 @@ class Pipeline: domain.id, composite_score=0.0, pass_reached=1, + commit=False, ) result.pass1_failed += 1 self._update_run_counters(run_id, result, current_pass=1) continue # p1_fail — skip remaining passes else: combo.status = "valid" - self.repo.update_combination_status(combo.id, "valid") + self.repo.update_combination_status(combo.id, "valid", commit=False) # Domain constraint check (per-domain block only). combo.status # stays "valid" here on purpose: it's domain-agnostic and the @@ -431,6 +451,7 @@ class Pipeline: domain_block_reason="; ".join( dc_result.violations ), + commit=False, ) result.pass1_failed += 1 self._update_run_counters( @@ -482,9 +503,13 @@ class Pipeline: "estimation_method": "llm" if self.llm else "stub", "confidence": 1.0, }) + # LLM-produced estimates commit immediately (slow/crash- + # prone, worth protecting); stub estimates are instant + # and defer, same as the rest of the deterministic passes. + used_llm = self.llm is not None if estimate_dicts: self.repo.save_raw_estimates( - combo.id, domain.id, estimate_dicts + combo.id, domain.id, estimate_dicts, commit=used_llm ) # Check for all-zero estimates → p2_fail @@ -492,11 +517,12 @@ class Pipeline: combo.status = "p2_fail" combo.block_reason = "All metric estimates are zero" self.repo.update_combination_status( - combo.id, "p2_fail", combo.block_reason + combo.id, "p2_fail", combo.block_reason, commit=used_llm ) self.repo.save_result( combo.id, domain.id, composite_score=0.0, pass_reached=2, + commit=used_llm, ) result.pass2_failed += 1 self._update_run_counters(run_id, result, current_pass=2) @@ -534,7 +560,7 @@ class Pipeline: "confidence": s.confidence, }) if score_dicts: - self.repo.save_scores(combo.id, domain.id, score_dicts) + self.repo.save_scores(combo.id, domain.id, score_dicts, commit=False) # Preserve existing human data novelty_flag = ( @@ -550,6 +576,7 @@ class Pipeline: sr.composite_score, pass_reached=3, novelty_flag=novelty_flag, human_notes=human_notes, + commit=False, ) combo.status = "p3_fail" combo.block_reason = ( @@ -557,7 +584,7 @@ class Pipeline: f"below threshold {score_threshold}" ) self.repo.update_combination_status( - combo.id, "p3_fail", combo.block_reason + combo.id, "p3_fail", combo.block_reason, commit=False ) result.pass3_failed += 1 result.pass3_scored += 1 @@ -571,8 +598,9 @@ class Pipeline: pass_reached=3, novelty_flag=novelty_flag, human_notes=human_notes, + commit=False, ) - self.repo.update_combination_status(combo.id, "scored") + self.repo.update_combination_status(combo.id, "scored", commit=False) result.pass3_scored += 1 result.pass3_above_threshold += 1 @@ -608,16 +636,21 @@ class Pipeline: for s in db_scores if s["normalized_score"] is not None } + raw_dict = { + s["metric_name"]: s["raw_value"] + for s in db_scores + if s["raw_value"] is not None + } review_result: tuple[str, bool] | None = None try: review_result = self.llm.review_plausibility( - description, score_dict + description, raw_dict, score_dict, domain.metric_bounds ) except LLMRateLimitError as exc: self._wait_for_rate_limit(run_id, exc.retry_after) try: review_result = self.llm.review_plausibility( - description, score_dict + description, raw_dict, score_dict, domain.metric_bounds ) except LLMRateLimitError: pass # still limited; skip, retry next run @@ -664,6 +697,13 @@ class Pipeline: ) result.top_results = self.repo.get_top_results(domain.name, limit=20) return result + finally: + # Flush any batched deterministic writes -- runs on normal + # completion, cancellation, and any other exception propagating + # out of the loop, so nothing deferred above is ever silently lost + # on a clean exit path (a hard process crash is a different story + # and is exactly what the immediate LLM-call commits protect). + self.repo.commit() # Mark run as completed if run_id is not None: @@ -842,15 +882,22 @@ class Pipeline: raw["power_density"] = (k_act * power_mass) / physics_denom if physics_denom else 0.0 if "range_fuel" in raw: - if storage_energy_form in AMBIENT_ENERGY_FORMS: + if storage_energy_form in AMBIENT_ENERGY_FORMS or k_med is None: + # Ambient sources aren't a depletable store (see module note + # above). Space/rocket platforms (k_med undeclared for + # "space") are the same conclusion from different physics: + # in vacuum coast there's no resistance to fight, so a + # working engine covers arbitrary distance given enough + # time -- "range" isn't fuel-quantity-limited the way it is + # for a vehicle fighting drag. The real constraint for a + # rocket is its delta-v budget (maneuvering capability), + # which isn't a distance and isn't what this metric asks -- + # reporting the domain's ceiling is the honest answer, not + # the old magic-constant guess (e_dens * 2.78) it replaces. mb = bounds_by_name.get("range_fuel") raw["range_fuel"] = mb.norm_max if mb else 0.0 - elif k_med is not None and floor_total > 0: + elif floor_total > 0: raw["range_fuel"] = min((e_dens * storage_mass) / (k_med * floor_total), 1e13) - else: - # space/rocket platforms: resistance-based formula doesn't - # apply (see module note) -- old placeholder, not a claim. - raw["range_fuel"] = min(e_dens * 2.78, 1e13) if "cost_efficiency" in raw: structural_cost = p_rep * STRUCTURAL_COST_PER_KG_BY_MEDIUM.get(medium, STRUCTURAL_COST_PER_KG_BY_MEDIUM["ground"]) diff --git a/src/physcom/llm/base.py b/src/physcom/llm/base.py index 34ee75a..91fe9cb 100644 --- a/src/physcom/llm/base.py +++ b/src/physcom/llm/base.py @@ -36,8 +36,20 @@ class LLMProvider(ABC): @abstractmethod def review_plausibility( - self, combination_description: str, scores: dict[str, float] + self, + combination_description: str, + raw_metrics: dict[str, float], + normalized_scores: dict[str, float], + metrics: list[MetricBound], ) -> tuple[str, bool]: - """Given a combination and its scores, return a (text, is_plausible) - tuple: natural-language assessment and whether the concept is plausible.""" + """Given a combination, its raw physical estimates, and their + normalized scores, return a (text, is_plausible) tuple: + natural-language assessment and whether the concept is plausible. + + Both raw_metrics and normalized_scores are given (not just the + normalized score) so the review can reason from the actual physics + rather than only a compressed 0-1 number, which can look + deceptively bad for a metric whose scale was built for a different + kind of vehicle. `metrics` carries each metric's unit for + formatting the raw value meaningfully.""" ... diff --git a/src/physcom/llm/prompts.py b/src/physcom/llm/prompts.py index 521cfb1..7173a7e 100644 --- a/src/physcom/llm/prompts.py +++ b/src/physcom/llm/prompts.py @@ -20,6 +20,35 @@ def format_metrics_for_prompt(metrics: list["MetricBound"]) -> str: return "\n".join(lines) +def format_scores_for_prompt( + raw_metrics: dict[str, float], + normalized_scores: dict[str, float], + metrics: list["MetricBound"], +) -> str: + """Render each metric with BOTH its raw physical value and its + normalized score, so the reviewing pass can reason from the actual + physics instead of only ever seeing a compressed 0-1 number. + + A real, correct estimate can still look damning once log-normalized + against a scale built for a different kind of vehicle (a cyclist's + real ~5 W/kg reads as "0.159" next to a car's 2000 W/kg ceiling) -- + a reviewer that only sees the 0.159 has no way to notice that. See + the labeled-set calibration note on PLAUSIBILITY_REVIEW_PROMPT below. + """ + lines = [] + for mb in metrics: + normed = normalized_scores.get(mb.metric_name) + if normed is None: + continue + raw = raw_metrics.get(mb.metric_name) + unit = mb.unit or "dimensionless" + raw_str = f"{raw:g} {unit}" if raw is not None else "unknown" + lines.append( + f"- {mb.metric_name}: raw estimate {raw_str} — normalized score {normed:.3f}" + ) + return "\n".join(lines) + + PHYSICS_ESTIMATION_PROMPT = """\ You are a physics estimation assistant. Given the following transportation concept, \ estimate the requested metrics using order-of-magnitude physics reasoning. @@ -44,15 +73,22 @@ match that magnitude, don't guess a generically "reasonable-looking" decimal. {{"some_metric": , "another_metric": }} — no explanatory text. """ -# ponytail: pass 4 only sees pass 2's raw numbers, not its reasoning. Sharpened -# prompts on both sides closed most of the gap (a bad safety estimate went from -# 0.95 to 0.80 on the same combo once pass 2 was told to consider combination- -# specific hazards). Upgrade path if this isn't good enough in practice: have -# estimate_physics() also return a short per-metric reason, persist it -# alongside raw_value (new nullable column), and feed it into this prompt so -# pass 4 has something concrete to agree or disagree with. Deferred because it -# needs a schema/interface change across LLMProvider + both providers + -# pipeline + scorer + repository, and more generated tokens per combo. +# ponytail: pass 4 used to see only pass 2's normalized scores, not the raw +# physical numbers or any reasoning behind them. Fixed the raw-value half of +# that gap: format_scores_for_prompt() now shows both, since a correct raw +# estimate can look damning once log-normalized against a scale built for a +# different kind of vehicle (a cyclist's real ~5 W/kg reads as "0.159" next +# to a car's 2000 W/kg ceiling) -- gemma2:27b did exactly this on a real +# bicycle combo, citing "extremely low power density (0.159)" as grounds for +# IMPLAUSIBLE while never reasoning from the actual (correct) 5 W/kg. The +# reasoning-text half of the gap is still open: estimate_physics() doesn't +# return a per-metric rationale, so pass 4 still can't see WHY pass 2 landed +# on a number, only what the number is. Upgrade path if the raw value alone +# isn't enough in practice: have estimate_physics() also return a short +# per-metric reason, persist it alongside raw_value (new nullable column), +# and feed it into this prompt. Deferred because it needs a schema/interface +# change across LLMProvider + both providers + pipeline + scorer + +# repository, and more generated tokens per combo. # # If we plan to LLM-review every p2 pass then maybe p2 and p4 should be combined. # @@ -84,10 +120,22 @@ is NOT the question. {description} ## Metric Scores -All scores below are normalized to 0-1, where HIGHER IS ALWAYS BETTER for -every metric listed, regardless of what the metric measures (this already -accounts for things like "lower cost is better" — you don't need to invert -anything). A score of 1.0 means excellent, not "pegged" or "maxed out badly." +Each metric below is given as its raw estimated physical value (in the unit +shown) AND a normalized score from 0-1, where HIGHER IS ALWAYS BETTER for +every metric listed regardless of what it measures (this already accounts +for things like "lower cost is better" — you don't need to invert anything). +A score of 1.0 means excellent, not "pegged" or "maxed out badly." + +Reason from the RAW value first — it's the actual physics. The normalized +score is a summary, not a fact on its own: a real, correct estimate can +still normalize to a low-looking number simply because the domain's scale +was built for a different, more demanding kind of vehicle (a cyclist's real +~5 W/kg legitimately normalizes to ~0.16 next to a car engine's 2000 W/kg +ceiling — that low score doesn't mean the estimate is bad or the concept is +weak, it means human power is small next to a car engine, which everyone +already knows). If a normalized score looks alarming, check whether the raw +value is actually reasonable for what this component fundamentally is +before treating the score as evidence of a problem. {scores} diff --git a/src/physcom/llm/providers/gemini.py b/src/physcom/llm/providers/gemini.py index b0af323..643b1e4 100644 --- a/src/physcom/llm/providers/gemini.py +++ b/src/physcom/llm/providers/gemini.py @@ -11,6 +11,7 @@ from physcom.llm.prompts import ( PHYSICS_ESTIMATION_PROMPT, PLAUSIBILITY_REVIEW_PROMPT, format_metrics_for_prompt, + format_scores_for_prompt, ) from physcom.models.domain import MetricBound @@ -46,9 +47,13 @@ class GeminiLLMProvider(LLMProvider): return parse_metric_json(response.text, metrics) def review_plausibility( - self, combination_description: str, scores: dict[str, float] + self, + combination_description: str, + raw_metrics: dict[str, float], + normalized_scores: dict[str, float], + metrics: list[MetricBound], ) -> tuple[str, bool]: - scores_str = "\n".join(f"- {k}: {v:.3f}" for k, v in scores.items()) + scores_str = format_scores_for_prompt(raw_metrics, normalized_scores, metrics) prompt = PLAUSIBILITY_REVIEW_PROMPT.format( description=combination_description, scores=scores_str, diff --git a/src/physcom/llm/providers/mock.py b/src/physcom/llm/providers/mock.py index f785757..832858e 100644 --- a/src/physcom/llm/providers/mock.py +++ b/src/physcom/llm/providers/mock.py @@ -21,9 +21,13 @@ class MockLLMProvider(LLMProvider): return result def review_plausibility( - self, combination_description: str, scores: dict[str, float] + self, + combination_description: str, + raw_metrics: dict[str, float], + normalized_scores: dict[str, float], + metrics: list[MetricBound], ) -> tuple[str, bool]: - avg = sum(scores.values()) / max(len(scores), 1) + avg = sum(normalized_scores.values()) / max(len(normalized_scores), 1) if avg > 0.5: return ("This concept appears plausible and worth further investigation.", True) return ("This concept has significant feasibility challenges.", False) diff --git a/src/physcom/llm/providers/ollama.py b/src/physcom/llm/providers/ollama.py index 2834c58..e2ef3a7 100644 --- a/src/physcom/llm/providers/ollama.py +++ b/src/physcom/llm/providers/ollama.py @@ -12,6 +12,7 @@ from physcom.llm.prompts import ( PHYSICS_ESTIMATION_PROMPT, PLAUSIBILITY_REVIEW_PROMPT, format_metrics_for_prompt, + format_scores_for_prompt, ) from physcom.models.domain import MetricBound @@ -34,9 +35,13 @@ class OllamaLLMProvider(LLMProvider): return parse_metric_json(text, metrics) def review_plausibility( - self, combination_description: str, scores: dict[str, float] + self, + combination_description: str, + raw_metrics: dict[str, float], + normalized_scores: dict[str, float], + metrics: list[MetricBound], ) -> tuple[str, bool]: - scores_str = "\n".join(f"- {k}: {v:.3f}" for k, v in scores.items()) + scores_str = format_scores_for_prompt(raw_metrics, normalized_scores, metrics) prompt = PLAUSIBILITY_REVIEW_PROMPT.format( description=combination_description, scores=scores_str,