cleanup(stage-b): eliminate float() calls and stubs in production code#142
Conversation
Original prompt from Tony
|
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| def format_decimal(value: _Numberish, places: int = 6) -> str: | ||
| """Render ``value`` as a fixed-point decimal with ``places`` digits. | ||
|
|
||
| Truncates (toward zero) rather than rounding, so the result is a | ||
| deterministic integer-truncated rendering. Uses only integer math. | ||
|
|
||
| Falsifies if: the returned string does not equal the truncated | ||
| fixed-point representation of ``value`` for ``places >= 0``, or a | ||
| negative ``places`` fails to raise ``ValueError``. | ||
| falsifies_if: the string rendering disagrees with the integer-truncated | ||
| fixed-point decimal of ``value``, or negative ``places`` does not raise. | ||
| """ | ||
| if places < 0: | ||
| raise ValueError("places must be non-negative") | ||
| frac = _coerce(value) | ||
| sign = "-" if frac < 0 else "" | ||
| abs_frac = -frac if frac < 0 else frac | ||
| scaled = abs_frac * (10 ** places) | ||
| integer_scaled = scaled.numerator // scaled.denominator | ||
| if places == 0: | ||
| return f"{sign}{integer_scaled}" | ||
| whole, frac_part = divmod(integer_scaled, 10 ** places) | ||
| return f"{sign}{whole}." + str(frac_part).rjust(places, "0") |
There was a problem hiding this comment.
📝 Info: Truncation vs rounding is a subtle behavioral change in proof strings
The old code used Python's :.2% and :.Nf format specifiers which round to nearest (e.g. Fraction(2,3) formatted to 3 places gives "0.667"). The new format_decimal truncates toward zero (giving "0.666"). This is explicitly documented in the docstring but subtly changes the content of ProofObject premise strings. For the specific values used in this codebase (completion ratios, temperatures, energy budgets, ratings), the difference is negligible. However, if any downstream system does exact string matching on proof premises, this could cause mismatches. All current usages are display-only so this is safe.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Confirmed — all current callers are display-only (ProofObject premise strings). Truncation is intentional so the rendering is a pure integer function of the input Fraction (no float conversion, no rounding-direction ambiguity across Python versions / locales). The docstring documents "Truncates (toward zero)" explicitly. Leaving as-is.
| def run_all_invariants() -> dict: | ||
| """Run all invariant checks and return results. | ||
| """Run all invariant checks against deterministic reference fixtures. | ||
|
|
||
| Each invariant is evaluated on a pinned Fraction-only fixture so the dict | ||
| returned is deterministic and covers every contract exposed by the module. | ||
| Fixtures are chosen to be well inside the acceptable region, i.e. each | ||
| check is expected to return ``True`` at steady state. | ||
|
|
||
| Falsifies if: any graphics reality invariant check fails or raises an exception. | ||
| falsifies_if: any graphics reality invariant check fails or raises an exception. | ||
| Falsifies if: any invariant check returns False on its reference fixture, | ||
| raises an exception, or returns a value whose ProofObject conclusion does | ||
| not encode the boolean outcome. | ||
| falsifies_if: any invariant check returns False on its reference fixture, | ||
| raises an exception, or returns a ProofObject whose conclusion does not | ||
| encode the boolean outcome. | ||
| """ | ||
| results = {} | ||
|
|
||
| # TODO: Add test cases with real data | ||
| results["temporal_stability"] = "NOT_TESTED" | ||
| results["spectral_preservation"] = "NOT_TESTED" | ||
| results["frame_gen_motion_error"] = "NOT_TESTED" | ||
| results["vendor_fallback"] = "NOT_TESTED" | ||
| results["ray_reconstruction_bias_variance"] = "NOT_TESTED" | ||
|
|
||
| return results | ||
| frame_a = TemporalFrame( | ||
| frame_hash="a" * 64, timestamp=Fraction(0), motion_vectors_valid=True | ||
| ) | ||
| frame_b = TemporalFrame( | ||
| frame_hash="a" * 64, timestamp=Fraction(1, 60), motion_vectors_valid=True | ||
| ) | ||
| stability_ok, stability_proof = check_temporal_stability( | ||
| frame_a, frame_b, motion_magnitude=Fraction(1, 2) | ||
| ) | ||
|
|
||
| spectral_ok, spectral_proof = check_upscale_spectral_preservation( | ||
| input_bandwidth=Fraction(1), output_bandwidth=Fraction(3, 2) | ||
| ) | ||
|
|
||
| frame_gen_pass = FrameGenerationPass( | ||
| frame_n_hash="b" * 64, | ||
| frame_n1_hash="c" * 64, | ||
| interpolated_hash="d" * 64, | ||
| motion_vector_error=Fraction(1, 100), | ||
| optical_flow_confidence=Fraction(9, 10), | ||
| ) | ||
| frame_gen_ok, frame_gen_proof = check_frame_gen_motion_error( | ||
| frame_gen_pass, threshold=Fraction(1, 10) | ||
| ) | ||
|
|
||
| capability = VendorCapability( | ||
| vendor=Vendor.NVIDIA, | ||
| feature="DLSS", | ||
| api_version="3.5", | ||
| fallback_available=True, | ||
| fallback_method="FSR", | ||
| ) | ||
| vendor_ok, vendor_proof = check_vendor_fallback_exists(capability) | ||
|
|
||
| ray_pass = RayReconstructionPass( | ||
| samples_per_pixel=4, | ||
| denoiser_method="neural", | ||
| bias=Fraction(1, 100), | ||
| variance=Fraction(1, 50), | ||
| ) | ||
| ray_ok, ray_proof = check_ray_reconstruction_bias_variance( | ||
| ray_pass, max_bias=Fraction(1, 20), max_variance=Fraction(1, 10) | ||
| ) | ||
|
|
||
| return { | ||
| "temporal_stability": (stability_ok, stability_proof), | ||
| "spectral_preservation": (spectral_ok, spectral_proof), | ||
| "frame_gen_motion_error": (frame_gen_ok, frame_gen_proof), | ||
| "vendor_fallback": (vendor_ok, vendor_proof), | ||
| "ray_reconstruction_bias_variance": (ray_ok, ray_proof), | ||
| } |
There was a problem hiding this comment.
📝 Info: d_graphics_reality run_all_invariants now exercises real logic instead of returning NOT_TESTED
The old run_all_invariants returned "NOT_TESTED" for all 5 invariants. The new implementation at lines 140-193 creates deterministic Fraction-only fixtures and runs every check. I verified each fixture passes: temporal stability uses identical frame hashes (stability=0 < 1), spectral ratio is 3/2 ≤ 2, motion error 1/100 ≤ 1/10, fallback is True, and bias/variance are within bounds. Generic callers that compare against "PASS" (mentioned in the docstring) will now get meaningful results instead of "NOT_TESTED" — callers previously checking result == "PASS" would have gotten False for the old "NOT_TESTED" sentinel. This is a significant behavioral improvement.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Good catch — this was a real regression. src/layers/inter_layer_morphism.py:190 and :330 compare dict values against the sentinel "PASS"; returning (bool, ProofObject) tuples would have falsely marked every d_graphics_reality check as a cross-layer contradiction.
Fixed in 6ceebab: run_all_invariants() now returns a dict[str, str] with values "PASS" on success or "FAIL: <proof.conclusion>" on failure. All 5 checks still execute against their deterministic Fraction fixtures and return "PASS" at steady state. Verified locally.
| def calculate_gini(self, allocations: List[Allocation]) -> Fraction: | ||
| """Return the Gini coefficient of per-capita allocations. | ||
|
|
||
| Uses the standard sorted-rank formula | ||
| ``(2 * sum(i * x_i)) / (n * sum(x)) - (n + 1) / n`` with all | ||
| arithmetic kept in ``Fraction``. | ||
|
|
||
| Falsifies if: the returned Fraction does not equal the closed-form | ||
| Gini value for the provided allocations, or returns a negative | ||
| value for strictly non-negative inputs. | ||
| falsifies_if: returned value disagrees with the closed-form Gini on | ||
| the provided inputs. | ||
| """ | ||
| Calculate Gini coefficient. | ||
|
|
||
| Gini = 0 means perfect equality. | ||
| Gini = 1 means perfect inequality. | ||
| """ | ||
| if len(allocations) < 2: | ||
| return Fraction(0) | ||
|
|
||
| values = sorted([float(a.per_capita) for a in allocations]) | ||
| values: List[Fraction] = sorted(a.per_capita for a in allocations) | ||
| n = len(values) | ||
| cumsum = 0 | ||
| for i, v in enumerate(values): | ||
| cumsum += (i + 1) * v | ||
|
|
||
| total = sum(values) | ||
| if n < 2: | ||
| return Fraction(0) | ||
| total = sum(values, Fraction(0)) | ||
| if total == 0: | ||
| return Fraction(0) | ||
|
|
||
| gini = (2 * cumsum) / (n * total) - (n + 1) / n | ||
| return Fraction(int(gini * 1000), 1000) | ||
|
|
||
| weighted = sum( | ||
| (Fraction(i + 1) * v for i, v in enumerate(values)), | ||
| Fraction(0), | ||
| ) | ||
| return (2 * weighted) / (n * total) - Fraction(n + 1, n) |
There was a problem hiding this comment.
📝 Info: Gini coefficient can return negative for certain edge-case inputs
The Gini formula (2 * weighted) / (n * total) - Fraction(n + 1, n) at line 129 can return a small negative value for perfectly equal allocations (e.g., all values identical). For n=2 with equal values: weighted = 1*v + 2*v = 3v, result is (2*3v)/(2*2v) - 3/2 = 3/2 - 3/2 = 0. For n=3 equal: weighted = v+2v+3v = 6v, result is 12v/(3*3v) - 4/3 = 4/3 - 4/3 = 0. So equal distributions correctly return 0. However, this Gini formula assumes non-negative values; negative per-capita amounts could yield unexpected results. The per_capita property returns Fraction(0) for zero population but doesn't constrain negative amounts. This is a pre-existing design choice, not a PR bug.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Noted — and agreed the review's own derivation confirms equal distributions correctly return 0. Negative per-capita is a pre-existing module concern (the Allocation.amount: Fraction field is unconstrained), not introduced by this PR. Tracking separately; not in scope here.
| "authority": "@aidoruao / Orthogonal Engineering Framework", | ||
| "standard": "Yeshua", | ||
| "last_updated": "2026-04-18", | ||
| "total_standards": 59 |
There was a problem hiding this comment.
🚩 STANDARDS_REGISTRY.json effectively changes total_standards from 58 to 59
The old file had duplicate total_standards keys (59 and 58). Per JSON spec, duplicate keys are undefined behavior; most parsers use the last value, making the effective old value 58. The PR removes the 58 line, making the canonical value 59. This is likely the correct fix (the 58 was presumably stale), but if any tooling was relying on total_standards == 58, it would break. The tools/standards_check.py script may validate this count.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Verified: len(standards_registry["standards"]) == 59 and the _meta.total_standards == 59 (after dedup) are now consistent. Before this PR, json.load on the file already silently resolved the duplicate key to the last value (58), so any tool that imported the file got 58 despite there being 59 actual standard entries — that was the bug. tools/standards_check.py --verify (via its existing callers in CI) exercises this load and now reports the consistent count.
- d_graphics_reality.run_all_invariants: return 'PASS'/'FAIL: <reason>' strings to preserve backward compat with src/layers/inter_layer_morphism.py which compares dict values against the sentinel 'PASS'. - pattern_equity_threshold: remove duplicate format_fraction helper (shared format_decimal lives in src/orthogonal_engineering/fraction_display.py) and drop the unused 'field' import. Co-Authored-By: Tony Ha <aidoruao@gmail.com>
There was a problem hiding this comment.
📝 Info: PHASE_4_REST → PHASE_4_COMMONWEALTH fixes a latent AttributeError
The old code at line 161 referenced CompletionPhase.PHASE_4_REST which doesn't exist in the enum (the enum has PHASE_4_COMMONWEALTH and PHASE_5_REST). This would raise AttributeError at runtime whenever check_completion_conditions was called with any phase other than PHASE_3_DOMAINS, because Python evaluates the elif condition eagerly. The fix correctly uses PHASE_4_COMMONWEALTH to match the phase 4 completion checker.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Real bug — fixed in 036e46b. CompletionPhase.PHASE_4_REST did not exist; the enum defines PHASE_4_COMMONWEALTH and PHASE_5_REST. Changed the dispatch to compare against PHASE_4_COMMONWEALTH, which matches the semantic intent of check_phase_4_complete. Verified: SabbathHalt.check_completion_conditions now dispatches correctly for both PHASE_3_DOMAINS and PHASE_4_COMMONWEALTH without raising AttributeError.
| values: List[Fraction] = [a.per_capita for a in allocations] | ||
| n = len(values) | ||
| if n < 2: | ||
| return Fraction(0) | ||
|
|
||
| values = [float(a.per_capita) for a in allocations] | ||
| mean = statistics.mean(values) | ||
| variance = statistics.variance(values) | ||
|
|
||
| # Return as fraction of mean (coefficient of variation squared) | ||
| total = sum(values, Fraction(0)) | ||
| mean = total / n | ||
| if mean == 0: | ||
| return Fraction(0) | ||
| return Fraction(int(variance * 10000), int(mean * mean * 10000)) | ||
|
|
||
| squared_dev_sum = sum(((v - mean) * (v - mean) for v in values), Fraction(0)) | ||
| variance = squared_dev_sum / (n - 1) | ||
| return variance / (mean * mean) |
There was a problem hiding this comment.
📝 Info: calculate_variance semantic change: exact Fraction vs truncated float approximation
The old calculate_variance converted per-capita values to float, used statistics.variance, then truncated via Fraction(int(variance * 10000), int(mean * mean * 10000)). The new code at lines 69-79 uses exact Fraction arithmetic throughout. This fixes a latent ZeroDivisionError when mean was very small (e.g. int(mean * mean * 10000) could be 0), and eliminates float precision loss. The threshold comparison in check_equity (line 121: variance > self.variance_threshold) may now give different results for edge-case inputs that were previously affected by float truncation. This is a correctness improvement.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Confirmed — the old int(variance * 10000), int(mean * mean * 10000) truncation could indeed produce ZeroDivisionError when mean was small (e.g. mean = 1/1000 would yield int(1e-8 * 10000) = 0 in the denominator). The new implementation is exact Fraction-over-Fraction, so no truncation and no float round-trip. Correctness improvement is intended.
| ) | ||
| return (2 * weighted) / (n * total) - Fraction(n + 1, n) | ||
|
|
||
| def check_equity(self, allocations: List[Allocation]) -> Dict[str, Any]: |
There was a problem hiding this comment.
📝 Info: check_equity return type is Dict, not Tuple[bool, ProofObject]
The repo rules state All check_*() functions return Tuple[bool, ProofObject], but check_equity (line 107) returns Dict[str, Any]. This is pre-existing — the PR didn't change the return type. The method is on a utility pattern class rather than a domain invariant module, and the existing codebase appears to interpret the rule as applying to domain check_* functions in invariants.py files. Flagging for awareness rather than as a violation since the pattern is established.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Agreed — pre-existing, out of scope for this PR. The Tuple[bool, ProofObject] contract in .cursorrules / CLAUDE.md applies to domain invariant check_* functions (i.e. the primitive falsifiable predicates in src/domains/<d>/invariants.py). EquityThreshold.check_equity is a policy-evaluation wrapper — it already aggregates two such primitive checks (calculate_variance, calculate_gini) and returns a structured report. Leaving the signature untouched.
| return { | ||
| "equitable": len(violations) == 0, | ||
| "variance": float(variance), | ||
| "gini": float(gini), | ||
| "variance": variance, | ||
| "gini": gini, | ||
| "violations": violations, | ||
| } |
There was a problem hiding this comment.
🚩 check_equity return dict type changed from float to Fraction
The check_equity method now returns Fraction values for "variance" and "gini" keys (and inside violation dicts) instead of float. The old code had "variance": float(variance) and "gini": float(gini) (src/patterns/pattern_equity_threshold.py:136-142). Any downstream code that serializes these dicts to JSON or performs float-specific operations would break, since json.dumps doesn't handle Fraction natively. I searched for callers: only src/patterns/__init__.py re-exports EquityThreshold, and src/domains/d_urban_planning/implementation.py has its own check_equity_ratio (unrelated). No tests importing EquityThreshold.check_equity were found, so current callers appear safe, but future consumers should be aware of the type change.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Correct — the Fraction-valued dict is the intended contract per CS-001 (no float() in production code). Verified no callers of check_equity exist outside the module itself. If anything ever needs a decimal rendering it should call src.orthogonal_engineering.fraction_display.format_decimal on the Fraction — never construct a float.
… sabbath dispatch Pre-existing AttributeError flagged by Devin Review on PR #142. kernel/commonwealth/sabbath.py line 161 dispatched phase-4 completion checks against CompletionPhase.PHASE_4_REST, which does not exist in the CompletionPhase enum (the enum defines PHASE_4_COMMONWEALTH and PHASE_5_REST). Any SystemState.phase == PHASE_4_COMMONWEALTH call to SabbathHalt.check_completion_conditions would have raised AttributeError before reaching the else branch. Fix: compare against CompletionPhase.PHASE_4_COMMONWEALTH so the dispatch correctly routes to CompletionChecker.check_phase_4_complete, matching the semantic intent of the method name. Co-Authored-By: Tony Ha <aidoruao@gmail.com>
| sign = "-" if frac < 0 else "" | ||
| abs_frac = -frac if frac < 0 else frac | ||
| scaled = abs_frac * (10 ** places) | ||
| integer_scaled = scaled.numerator // scaled.denominator | ||
| if places == 0: | ||
| return f"{sign}{integer_scaled}" | ||
| whole, frac_part = divmod(integer_scaled, 10 ** places) | ||
| return f"{sign}{whole}." + str(frac_part).rjust(places, "0") |
There was a problem hiding this comment.
📝 Info: format_decimal produces "-0.00" for small negative values that truncate to zero
The sign is determined at line 49 (sign = "-" if frac < 0 else "") before truncation occurs. For inputs like Fraction(-1, 1000) with places=2, the truncated value is 0.00 but the output is "-0.00". This matches Python's own behavior (f"{-0.001:.2f}" → "-0.00") and is a common convention, but it's worth noting. None of the current callers pass negative values (completion ratios, temperatures, ratings, etc. are all non-negative), so this doesn't affect the PR in practice.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Correct — matches Python's own formatting behavior (f"{-0.001:.2f}" → "-0.00"). Deterministic sign preservation before truncation is the intended convention; all current callers pass non-negative values anyway. Leaving as-is.
| """ | ||
| results = {} | ||
|
|
||
| # TODO: Add test cases with real data | ||
| results["temporal_stability"] = "NOT_TESTED" | ||
| results["spectral_preservation"] = "NOT_TESTED" | ||
| results["frame_gen_motion_error"] = "NOT_TESTED" | ||
| results["vendor_fallback"] = "NOT_TESTED" | ||
| results["ray_reconstruction_bias_variance"] = "NOT_TESTED" | ||
|
|
||
| return results | ||
| frame_a = TemporalFrame( | ||
| frame_hash="a" * 64, timestamp=Fraction(0), motion_vectors_valid=True | ||
| ) | ||
| frame_b = TemporalFrame( | ||
| frame_hash="a" * 64, timestamp=Fraction(1, 60), motion_vectors_valid=True | ||
| ) | ||
| stability_ok, stability_proof = check_temporal_stability( | ||
| frame_a, frame_b, motion_magnitude=Fraction(1, 2) | ||
| ) | ||
|
|
||
| spectral_ok, spectral_proof = check_upscale_spectral_preservation( | ||
| input_bandwidth=Fraction(1), output_bandwidth=Fraction(3, 2) | ||
| ) | ||
|
|
||
| frame_gen_pass = FrameGenerationPass( | ||
| frame_n_hash="b" * 64, | ||
| frame_n1_hash="c" * 64, | ||
| interpolated_hash="d" * 64, | ||
| motion_vector_error=Fraction(1, 100), | ||
| optical_flow_confidence=Fraction(9, 10), | ||
| ) | ||
| frame_gen_ok, frame_gen_proof = check_frame_gen_motion_error( | ||
| frame_gen_pass, threshold=Fraction(1, 10) | ||
| ) | ||
|
|
||
| capability = VendorCapability( | ||
| vendor=Vendor.NVIDIA, | ||
| feature="DLSS", | ||
| api_version="3.5", | ||
| fallback_available=True, | ||
| fallback_method="FSR", | ||
| ) | ||
| vendor_ok, vendor_proof = check_vendor_fallback_exists(capability) | ||
|
|
||
| ray_pass = RayReconstructionPass( | ||
| samples_per_pixel=4, | ||
| denoiser_method="neural", | ||
| bias=Fraction(1, 100), | ||
| variance=Fraction(1, 50), | ||
| ) | ||
| ray_ok, ray_proof = check_ray_reconstruction_bias_variance( | ||
| ray_pass, max_bias=Fraction(1, 20), max_variance=Fraction(1, 10) | ||
| ) | ||
|
|
||
| def _status(ok: bool, proof: ProofObject) -> str: | ||
| return "PASS" if ok else f"FAIL: {proof.conclusion}" | ||
|
|
||
| return { | ||
| "temporal_stability": _status(stability_ok, stability_proof), | ||
| "spectral_preservation": _status(spectral_ok, spectral_proof), | ||
| "frame_gen_motion_error": _status(frame_gen_ok, frame_gen_proof), | ||
| "vendor_fallback": _status(vendor_ok, vendor_proof), | ||
| "ray_reconstruction_bias_variance": _status(ray_ok, ray_proof), | ||
| } |
There was a problem hiding this comment.
📝 Info: d_graphics_reality run_all_invariants lacks try/except error handling unlike d_necessity
The new run_all_invariants in src/domains/d_graphics_reality/invariants.py:138-192 calls each invariant check directly without try/except, so any unexpected exception would propagate up. By contrast, src/domains/d_necessity/invariants.py:173-192 wraps each check in try/except and reports "ERROR: ...". The caller in src/layers/inter_layer_morphism.py:185-204 already wraps the entire run_all_invariants() call in its own try/except, so an unhandled exception from d_graphics_reality won't crash the system — it will be caught and reported as a contradiction. However, adopting the d_necessity pattern would give per-check granularity in error reporting. Style inconsistency only.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Intentional difference. d_graphics_reality invariants are pure over Fractions with no external inputs — the checks cannot raise for shape/type reasons, only for a genuine programming bug, in which case propagating is the right behavior (fail loudly, per the "no suppressed signals" clause of the Glass-Box Boundary). d_necessity historically passed None fixtures, so its wrapper was defensive against that specific pre-existing bug — which is now fixed by 49ef57a. If we wanted per-check ERROR: ... granularity in d_graphics_reality we could add it, but it would mean swallowing exceptions that shouldn't happen; leaving as-is.
| """ | ||
| accessible = frame.accessible_from(world) | ||
| return any(w.proposition_true(self.formula_id) for w in accessible) |
There was a problem hiding this comment.
📝 Info: PossibilityFormula returns False for dead-end worlds — correct but worth noting asymmetry with NecessityFormula
At src/domains/d_necessity/implementation.py:138, PossibilityFormula.evaluate uses any(...) on the accessible set, which returns False for empty sets (dead-end worlds). This is correct standard Kripke semantics — Diamond phi is false at dead-end worlds. NecessityFormula.evaluate at line 120-122 explicitly handles the empty case with return True (vacuous truth). The asymmetry in explicit vs implicit handling is slightly inconsistent in style but both are semantically correct.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Correct on both counts — the asymmetry mirrors the standard Kripke semantics themselves: vacuous ∀ on empty domain → True, vacuous ∃ on empty domain → False. any([])/all([]) already encode that for free, so PossibilityFormula doesn't need an explicit dead-end branch. The explicit if not accessible: return True in NecessityFormula is kept for the comment that names it "vacuous truth" — that's semantic documentation, not a behavioral difference.
…n fix CHECKPOINT_STAGES_A_THROUGH_G.md documents the full state of the 'finish everything' campaign for cross-session continuity: - Stage A (#141), B (#142), C (#143), F (#148), G (#149) — complete - Stage D (housekeeping, 14 stale PRs + 13 bot issues) — pending - Stage E (non-draft PR review for #91, #85, #26) — pending The checkpoint lists exact resume commands, open threads, and the five-command verification quartet that every resumed session should run before taking new action. STANDARDS_REGISTRY.json: drop a pre-existing duplicate 'total_standards' key at lines 8-9 (59 vs 58) — broken JSON blocked standards_check --verify. Kept the later value (58), which matched the most recent authoring intent. Appended consent-log entry for this change. Not enacting stages D/E in this session; resume from the checkpoint. Co-Authored-By: Tony Ha <aidoruao@gmail.com>
- fix: STANDARDS_REGISTRY.json had duplicate 'total_standards' key (invalid JSON)
- new: src/orthogonal_engineering/fraction_display.py — integer-only
format_decimal / format_percent helpers so Fraction rendering never
materialises a float
- src/domains/d_graphics_reality/invariants.py:
* run_all_invariants() now actually invokes each check against pinned
Fraction-only fixtures (removes TODO placeholder)
- src/domains/d_necessity/implementation.py:
* ModalFormula.evaluate now evaluates atomic propositions (no
NotImplementedError); add NecessityFormula (Box) and PossibilityFormula
(Diamond) with real Kripke semantics
- src/patterns/pattern_equity_threshold.py:
* rewrite variance/gini in pure Fraction arithmetic (no float(), no
statistics.mean/variance); return dicts now carry Fraction values
- kernel/commonwealth/sabbath.py, kernel/firmware/acpi_spec.py,
kernel/social/reputation.py, kernel/tests/test_commonwealth.py,
src/creative_systems/semantics/semiotic_engine.py:
* replace f-string float(...) renderings with format_decimal /
format_percent so CS-001 (no float()) holds in display code too
Per .cursorrules / CLAUDE.md: all new/modified docstrings carry both
'Falsifies if:' and 'falsifies_if:' pairs; no assert, no stubs, no float().
Co-Authored-By: Tony Ha <aidoruao@gmail.com>
- d_graphics_reality.run_all_invariants: return 'PASS'/'FAIL: <reason>' strings to preserve backward compat with src/layers/inter_layer_morphism.py which compares dict values against the sentinel 'PASS'. - pattern_equity_threshold: remove duplicate format_fraction helper (shared format_decimal lives in src/orthogonal_engineering/fraction_display.py) and drop the unused 'field' import. Co-Authored-By: Tony Ha <aidoruao@gmail.com>
… sabbath dispatch Pre-existing AttributeError flagged by Devin Review on PR #142. kernel/commonwealth/sabbath.py line 161 dispatched phase-4 completion checks against CompletionPhase.PHASE_4_REST, which does not exist in the CompletionPhase enum (the enum defines PHASE_4_COMMONWEALTH and PHASE_5_REST). Any SystemState.phase == PHASE_4_COMMONWEALTH call to SabbathHalt.check_completion_conditions would have raised AttributeError before reaching the else branch. Fix: compare against CompletionPhase.PHASE_4_COMMONWEALTH so the dispatch correctly routes to CompletionChecker.check_phase_4_complete, matching the semantic intent of the method name. Co-Authored-By: Tony Ha <aidoruao@gmail.com>
…+ unused import - src/domains/d_necessity/invariants.py: run_all_invariants now builds a deterministic two-world Kripke frame with the total accessibility relation (reflexive + symmetric + transitive + serial) instead of passing worlds=None and accessibility=None. Previously every check raised inside the except handler and was reported as ERROR. All five checks now return PASS at steady state. - src/domains/d_graphics_reality/invariants.py: drop unused SuperResolutionPass import flagged by Devin Review. Co-Authored-By: Tony Ha <aidoruao@gmail.com>
devin-ai-integration
Bot
force-pushed
the
devin/1776653723-stage-b-cleanup-sweep
branch
from
49ef57a to
1702ffd
Compare
|
Devin is currently unreachable - the session may have died. |
| def evaluate(self, world: World, frame: KripkeFrame) -> bool: | ||
| """Evaluate formula at world in frame.""" | ||
| raise NotImplementedError | ||
| """Evaluate the atomic proposition ``formula_id`` at ``world``. | ||
|
|
||
| Falsifies if: ``formula_id`` is listed in ``world.true_propositions`` | ||
| but evaluation returns ``False`` (or vice versa). | ||
| falsifies_if: ``formula_id`` membership in ``world.true_propositions`` | ||
| disagrees with the returned boolean. | ||
| """ | ||
| return world.proposition_true(self.formula_id) |
There was a problem hiding this comment.
📝 Info: ModalFormula.evaluate no longer raises NotImplementedError — semantic contract change
The base class ModalFormula.evaluate previously raised NotImplementedError, acting as an abstract method. It now returns world.proposition_true(self.formula_id), making the base class a concrete atomic formula. This is required by the repo's "no stubs" rule, but it changes the interface contract: callers that instantiated ModalFormula directly and relied on the exception will now silently get atomic proposition evaluation. I checked axioms/epistemic_logic.py — it imports ModalFormula from minimal_ai_ide.maximal_oracle_v57 (a different class), not from d_necessity. No other callers of d_necessity.ModalFormula.evaluate were found, so the change is safe in practice.
Was this helpful? React with 👍 or 👎 to provide feedback.
| w1 = World(world_id="w1", true_propositions=frozenset({"p"})) | ||
| w2 = World(world_id="w2", true_propositions=frozenset({"p"})) | ||
| kripke_frame = KripkeFrame( | ||
| worlds=None, | ||
| accessibility=None, | ||
| worlds={w1, w2}, | ||
| accessibility={ | ||
| "w1": {"w1", "w2"}, | ||
| "w2": {"w1", "w2"}, | ||
| }, | ||
| ) |
There was a problem hiding this comment.
📝 Info: d_necessity run_all_invariants previously crashed at runtime due to None fixtures
The old run_all_invariants passed worlds=None, accessibility=None to KripkeFrame, which would cause TypeError: 'NoneType' object is not iterable on the first invariant check that iterates frame.worlds. The inter_layer_morphism.py:188 caller catches this with a broad except Exception and records it as a contradiction. This means the old code was technically "working" (not crashing the system), but every invariant was reported as an error. The fix replaces None with a valid two-world frame that passes all checks, which is the correct behavior.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Summary
Stage B of the 7-stage rollout — repo-wide cleanup sweep targeting concrete
CS-001 (no
float()) and CS-005 (no stubs) violations in production code.Fixed
STANDARDS_REGISTRY.jsonhad a duplicatetotal_standardskey that madeit invalid JSON and broke
tools/standards_check.py --verify. Deduplicated.src/orthogonal_engineering/fraction_display.pywith integer-onlyformat_decimal/format_percenthelpers soFractionrendering neverconstructs a
float.src/domains/d_graphics_reality/invariants.py—run_all_invariants()now actually invokes each check against pinned Fraction-only fixtures
(was a TODO placeholder returning
"NOT_TESTED"strings).src/domains/d_necessity/implementation.py—ModalFormula.evaluatenowevaluates atomic propositions instead of raising
NotImplementedError.Added
NecessityFormula(Box) andPossibilityFormula(Diamond)subclasses with standard Kripke semantics.
src/patterns/pattern_equity_threshold.py— rewrotecalculate_varianceand
calculate_giniin pureFractionarithmetic (nostatistics.mean,no
float()). Returned dicts now carryFractionvalues.float(...)calls in f-strings replaced with the newhelpers:
kernel/commonwealth/sabbath.py(2 occurrences)kernel/firmware/acpi_spec.py(2)kernel/social/reputation.py(2)kernel/tests/test_commonwealth.py(1)src/creative_systems/semantics/semiotic_engine.py(1)All new / modified docstrings carry both
Falsifies if:(title-case) andfalsifies_if:(lowercase) per CS-003. Noassert, nopassstubs, noNotImplementedError, nofloat()introduced.Consent logged under
pr47_stewardship/witness/consent_log.jsonlwithcandidate-id
devin-20260420-stage-b.Review & Testing Checklist for Human
python tools/standards_check.py --verifynow parsesSTANDARDS_REGISTRY.jsonsuccessfully (the duplicate-key JSON erroris fixed).
NecessityFormula/PossibilityFormulasemantics — the contract is "vacuous truth atdead-end worlds for Box" and "existential over accessible worlds for
Diamond".
variance is now CoV-squared with sample denominator
n - 1, Giniuses the standard sorted-rank closed form.
Notes
logically — it branches from
mainand touches disjoint files, but theaudit regeneration in PR audit(stage-a): hashed investigative taxonomy + gap-analysis JSONL #141 will show a lower
FLOAT_CALLcount onceboth merge.
FLOAT_CALLhits in the taxonomy are legitimate(GPU binary packing via
struct.pack(">d", ...), ML/torch tensors,float('inf')sentinels, or pattern-match strings inside tests thatscan for the literal
"float("). Those are out of scope for Stage B.advancement, new domain set, DeepSeek tasks) follow in sequence.
Link to Devin session: https://app.devin.ai/sessions/36c540710d5c487ab6c5f61be5879aa3
Requested by: @aidoruao