planner: fill the 5 default_modulation style families from canonical (TD-PLANNER-STYLE-DEFAULT-DRIFT-1 PAID)#713
Conversation
…(TD-PLANNER-STYLE-DEFAULT-DRIFT-1) D-TSC-1b measured that the planner's 5 `_ => FieldModulation::default()` style families (Convergent/Systematic/Divergent/Diffuse/Peripheral) diverge from the canonical UNIFIED_STYLES == thinking-engine StyleParams tables on the three shared dims (resonance/fan_out/exploration) — the sole drift among the 12. Replace the `_` fallback with 5 explicit arms carrying the canonical values on those 3 measured dims; the 4 planner-specific dims (depth/breadth/noise/speed bias) have no canonical source, so they stay at FieldModulation::default() via struct-update — only what was measured is filled, nothing fabricated. The match is now exhaustive over all 12 StyleFamily variants: the `_ => default` arm was exactly what let the drift go silent, so removing it makes a future unhandled variant a compile error instead of another silent default. +2 regression tests (canonical values on the 5 families' 3 dims + the 4 unmeasured dims stayed at default; exhaustiveness). 218 planner lib tests green (+2), clippy -D warnings exit 0. Behavior change (correctness): the 5 families now search with their real per-family params instead of the flat default. Numeric half of census O5 answered. Board: TECH_DEBT PAID, EPIPHANIES E-PLANNER-STYLE-DRIFT-PAID-1, AGENT_LOG. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Awg6TXocHcwTtc6eGsHcdD
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_6dd72efa-478e-41b6-9d3c-b68d9838606f) |
|
Warning Review limit reached
Next review available in: 44 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a75b16f1c9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| }, | ||
| Self::Peripheral => FieldModulation { | ||
| resonance_threshold: 0.20, | ||
| fan_out: 20, |
There was a problem hiding this comment.
When callers use the public plan_with_style(..., StyleFamily::Peripheral) path, api.rs converts this modulation through to_fingerprint(), which normalizes fan_out as fan_out as f64 / 20.0; with this new value that passes exactly 1.0 into thermometer_encode, where (1u8 << 8) panics in debug builds and encodes incorrectly in release builds. Peripheral previously fell back to fan_out = 6, so this change adds a new user-visible panic/wrong strategy-vector case unless the encoder is fixed or this value is kept below the saturation boundary.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed at the root cause in b3db4df. You're right that Peripheral fan_out = 20 → to_fingerprint → 20/20.0 = 1.0 → thermometer_encode(1.0) overflows. But the bug is in the encoder, not the value: thermometer_encode did (1u8 << levels).wrapping_sub(1) with levels = (value*8) as u8 ∈ 0..=8, and 1u8 << 8 overflows u8 (debug panic / release mis-encode to 0). It was already latent — Exploratory (exploration = 1.0, fan_out = 20) and Focused (depth_bias = 1.0) feed 1.0 through the same path, so plan_with_style(Exploratory) already panicked in debug; my Peripheral change just added another trigger on the reviewed line.
Rather than keep Peripheral below the boundary (which would reintroduce the D-TSC-1b drift this PR fixes), I fixed the encoder: ((1u16 << levels) - 1) as u8, so the top bin is a correct 0xFF (8 set bits) instead of overflowing. Added to_fingerprint_never_panics_for_any_family (exercises the real plan_with_style path across all 12 families — would have caught the pre-existing bug) + thermometer_encode_saturates_at_full_scale. 220 planner lib tests green.
Generated by Claude Code
…x P2 on #713) to_fingerprint() feeds each modulation dim through thermometer_encode, which computed (1u8 << levels).wrapping_sub(1) with levels = (value*8) as u8 in 0..=8. At value == 1.0, levels == 8 and `1u8 << 8` is a shift overflow: PANIC in debug, mis-encode to 0 in release. This was already latent for shipped families whose dims hit 1.0 (Exploratory exploration=1.0 / fan_out=20, Focused depth_bias=1.0), so plan_with_style(Exploratory) already panicked in debug; filling Peripheral fan_out=20 added another trigger on the reviewed line. Compute the thermometer code one width up: ((1u16 << levels) - 1) as u8, so the top bin is a correct 0xFF. +2 regression tests: thermometer_encode_saturates_ at_full_scale and to_fingerprint_never_panics_for_any_family (the latter would have caught the pre-existing bug). 220 planner lib green (+2), clippy -D exit 0. Board: EPIPHANIES E-THERMOMETER-ENCODE-OVERFLOW-1. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Awg6TXocHcwTtc6eGsHcdD
2 participants
What
Pays down the debt the D-TSC-1b agreement probe (#712) measured: the planner's
default_modulationleft 5 style families to_ => FieldModulation::default()(flat0.7/6/0.3), and that flat default was the sole measured divergence from the canonicalUNIFIED_STYLES≡ thinking-engineStyleParamstables (the driver + engine agree perfectly; the planner's 7 explicit families agree at IDENTITY).The change
crates/lance-graph-planner/src/thinking/style.rs— replace the_fallback with 5 explicit arms:sourced from the canonical table. Measured-only discipline: only the 3 dims the probe measured are filled; the 4 planner-specific dims (
depth_bias/breadth_bias/noise_tolerance/speed_bias, no canonical source) stay atFieldModulation::default()via..default()struct-update — nothing fabricated.The
_fallback is removed → the match is exhaustive over all 12StyleFamilyvariants. That fallback is exactly what let the drift go silent; removing it makes a future unhandled variant a compile error instead of another silent default.Verification
cargo test -p lance-graph-planner→ 218 lib (+2) green. New:formerly_defaulted_families_match_canonical_on_measured_dims(pins the 5 families' 3 dims + asserts the 4 unmeasured dims stayed at default) andevery_family_has_a_nondefault_or_explicit_modulation(exhaustiveness).cargo clippy -p lance-graph-planner --all-targets -- -D warnings→ exit 0.cargo fmtclean.Behavior change
Correctness: the 5 families now search with their real per-family resonance/fan_out/exploration instead of the flat default. Re-running D-TSC-1b would show Mode A ≈ Mode B. Existing 7 explicit arms untouched (byte-identical). Numeric half of census question O5 answered.
Board hygiene (same commit)
TD-PLANNER-STYLE-DEFAULT-DRIFT-1marked PAID; EPIPHANIESE-PLANNER-STYLE-DRIFT-PAID-1; AGENT_LOG.🤖 Generated with Claude Code
https://claude.ai/code/session_01Awg6TXocHcwTtc6eGsHcdD
Generated by Claude Code