Repository navigation
feat(safety): bed adhesion precondition gate - #51
Merged
Merged
Conversation
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
📝 WalkthroughWalkthroughA new P2 bed-adhesion safety gate is introduced to validate first-layer Z offset tolerance and minimum bed temperature fraction before homing. The precondition check is integrated into ChangesBed-Adhesion Safety Gate
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Owner
Author
|
Fixed the fresh correctness-audit finding: z_tolerance_mm and min_bed_fraction now reject non-finite threshold values, with focused regression coverage. Local focused tests: 20 passed; pre-push fast subset passed. |
Owner
Author
|
Fixed the second correctness-audit finding: room-temperature bed profiles (�ed_target_c=0) now pass the bed-adhesion temperature precondition while negative bed targets still fail. Added focused and bundle regression coverage. Local focused tests: 22 passed; safety slice: 66 passed; pre-push fast subset passed. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@03_implementation/src/hermes3d/core/safety/bed_adhesion.py`:
- Around line 62-92: The function check_bed_adhesion_precondition currently
treats homed=None as "not homed" and allows the check to pass; change it to fail
closed by explicitly detecting homed is None and appending a rejection reason
(e.g., "homing state unknown") so the precondition gate rejects unknown homing
state; locate the homed parameter use in check_bed_adhesion_precondition and add
a branch that appends the failure reason when homed is None (keep the existing
branch that appends "bed adhesion precondition must run before homing" when
homed is True).
- Around line 46-59: The to_dict method may emit NaN/Inf for numeric telemetry;
update the to_dict in the bed adhesion class to sanitize all numeric fields
(first_layer_z_offset_mm, target_first_layer_z_offset_mm, z_delta_mm,
z_tolerance_mm, bed_actual_c, bed_target_c, bed_min_c, min_bed_fraction, and any
other numeric attributes) by checking math.isfinite(...) and replacing
non-finite values with a JSON-safe sentinel (e.g., None) before placing them in
the returned dict; keep boolean/string fields (passed, homed, reasons) unchanged
and preserve the dict keys exactly as in the current to_dict signature.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 16f4756b-c3d6-44f2-9a8e-3fced9b2529a
📒 Files selected for processing (4)
03_implementation/src/hermes3d/core/safety/__init__.py03_implementation/src/hermes3d/core/safety/bed_adhesion.py04_testing/pytest/test_safety_bed_adhesion.py04_testing/pytest/test_safety_bundle.py
Comment on lines
+46
to
+59
| def to_dict(self) -> dict[str, Any]: | ||
| return { | ||
| "passed": self.passed, | ||
| "first_layer_z_offset_mm": self.first_layer_z_offset_mm, | ||
| "target_first_layer_z_offset_mm": self.target_first_layer_z_offset_mm, | ||
| "z_delta_mm": self.z_delta_mm, | ||
| "z_tolerance_mm": self.z_tolerance_mm, | ||
| "bed_actual_c": self.bed_actual_c, | ||
| "bed_target_c": self.bed_target_c, | ||
| "bed_min_c": self.bed_min_c, | ||
| "min_bed_fraction": self.min_bed_fraction, | ||
| "homed": self.homed, | ||
| "reasons": list(self.reasons), | ||
| } |
There was a problem hiding this comment.
Normalize non-finite telemetry before to_dict().
to_dict() can emit raw nan/inf values from failed checks. That makes the payload unsafe for downstream serialization and violates the module’s pure-data contract.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@03_implementation/src/hermes3d/core/safety/bed_adhesion.py` around lines 46 -
59, The to_dict method may emit NaN/Inf for numeric telemetry; update the
to_dict in the bed adhesion class to sanitize all numeric fields
(first_layer_z_offset_mm, target_first_layer_z_offset_mm, z_delta_mm,
z_tolerance_mm, bed_actual_c, bed_target_c, bed_min_c, min_bed_fraction, and any
other numeric attributes) by checking math.isfinite(...) and replacing
non-finite values with a JSON-safe sentinel (e.g., None) before placing them in
the returned dict; keep boolean/string fields (passed, homed, reasons) unchanged
and preserve the dict keys exactly as in the current to_dict signature.
Comment on lines
+62
to
+92
| def check_bed_adhesion_precondition( | ||
| *, | ||
| first_layer_z_offset_mm: float, | ||
| target_first_layer_z_offset_mm: float, | ||
| bed_actual_c: float, | ||
| bed_target_c: float, | ||
| z_tolerance_mm: float = 0.05, | ||
| min_bed_fraction: float = 0.97, | ||
| homed: bool | None = None, | ||
| ) -> BedAdhesionCheckResult: | ||
| """Validate bed-adhesion preconditions before homing or upload.""" | ||
| reasons: list[str] = [] | ||
|
|
||
| if not math.isfinite(z_tolerance_mm) or z_tolerance_mm < 0: | ||
| raise ValueError("z_tolerance_mm must be finite and non-negative") | ||
| if not math.isfinite(min_bed_fraction) or not (0 < min_bed_fraction <= 1): | ||
| raise ValueError("min_bed_fraction must be in (0, 1]") | ||
|
|
||
| inputs = { | ||
| "first_layer_z_offset_mm": first_layer_z_offset_mm, | ||
| "target_first_layer_z_offset_mm": target_first_layer_z_offset_mm, | ||
| "bed_actual_c": bed_actual_c, | ||
| "bed_target_c": bed_target_c, | ||
| } | ||
| for label, value in inputs.items(): | ||
| if not math.isfinite(value): | ||
| reasons.append(f"{label} must be finite") | ||
|
|
||
| if homed is True: | ||
| reasons.append("bed adhesion precondition must run before homing") | ||
|
|
There was a problem hiding this comment.
Fail closed when homing state is unknown.
homed=None currently behaves like “not homed,” so a caller can omit the new flag and still pass the gate. For a safety precondition that is explicitly meant to guard pre-homing behavior, unknown homing state should be rejected instead of silently accepted.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@03_implementation/src/hermes3d/core/safety/bed_adhesion.py` around lines 62 -
92, The function check_bed_adhesion_precondition currently treats homed=None as
"not homed" and allows the check to pass; change it to fail closed by explicitly
detecting homed is None and appending a rejection reason (e.g., "homing state
unknown") so the precondition gate rejects unknown homing state; locate the
homed parameter use in check_bed_adhesion_precondition and add a branch that
appends the failure reason when homed is None (keep the existing branch that
appends "bed adhesion precondition must run before homing" when homed is True).
6 tasks done
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
safety.bed_adhesion_preconditiongate for first-layer Z offset and bed-temperature readiness before homing/uploadrun_all_safety_gatesand the public safety registrySafety / local settings
.envcontents, or user setup values are committedAudit fixes
z_tolerance_mm/min_bed_fractionthreshold values so thresholds fail closedbed_target_c=0.0room-temperature/no-heated-bed profiles while still rejecting negative bed targetsVerification
python -m pytest 04_testing/pytest/test_safety_bed_adhesion.py 04_testing/pytest/test_safety_bundle.py -q— 22 passedpython -m pytest 04_testing/pytest/test_safety_bed_adhesion.py 04_testing/pytest/test_safety_bundle.py 04_testing/pytest/test_safety_emergency_stop.py 04_testing/pytest/test_safety_gcode_bounds.py 04_testing/pytest/test_safety_material_window.py 04_testing/pytest/test_safety_thermal_runaway.py -q— 66 passedpython -m pytest 04_testing/pytest -q— passed before the audit-fix commits; latest head is covered by GitHub CI plus focused/safety reruns abovepython -m ruff check 03_implementation/src 04_testing/pytest— passed before audit fixes; changed-file ruff reruns passed after fixespython -m ruff format --check 03_implementation/src 04_testing/pytest— passed before audit fixes; changed-file format checks passed after fixesHermes task:
H3D-BED-ADHESION-PRECONDITION-GATE