fix(lineage): require exact calibrated active channel profiles - #638
Conversation
…ain' into fix/exact-active-channel-weights
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
3609d63
into
feat/external-lineage-contract-main
![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: 51c2be9d3e
ℹ️ 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".
| if set(weights) != expected: | ||
| raise ValueError("weights must exactly match the active lineage channels") |
There was a problem hiding this comment.
When llm is absent or reports available = False, callers supplying a vector that includes llm now receive ValueError instead of reconstructing with that unavailable channel removed. Restore dropping llm and renormalizing the remaining weights, as required for NullAdjudicationClient; otherwise the documented missing-signal fallback is broken.
AGENTS.md reference: AGENTS.md:L189-L195
Useful? React with 👍 / 👎.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| expected = set(_CORE_WEIGHT_CHANNELS) | ||
| if getattr(llm, "available", False): | ||
| expected.add("llm") | ||
| if set(weights) != expected: | ||
| raise ValueError("weights must exactly match the active lineage channels") | ||
| values = tuple(weights.values()) | ||
| if any( | ||
| isinstance(value, bool) | ||
| or not isinstance(value, (int, float)) | ||
| or not math.isfinite(float(value)) | ||
| or float(value) <= 0.0 | ||
| for value in values | ||
| ) or not math.isclose(sum(values), 1.0, abs_tol=1e-9): | ||
| raise ValueError("active lineage weights must be finite, positive, and sum to one") | ||
| return dict(weights) |
There was a problem hiding this comment.
🟡 Code contradicts the mandatory pluggable-channel convention
active_weights now rejects a mismatched channel set with ValueError instead of dropping and renormalizing the unavailable channel. The AGENTS.md "Pluggable channels" section still mandates the drop-and-renormalize behavior and names this exact function, and it was not updated to match.
Prompt for agents
The mandatory convention file AGENTS.md (section "Pluggable channels: never fake a missing signal") states that Null channel clients must "make their channel dropped + renormalized (reconstruct.active_weights)". This PR intentionally changes reconstruct.active_weights to require an exact calibrated channel vector and raise ValueError instead of dropping/renormalizing, per the ADR 0200/0205 amendments. Update AGENTS.md to reflect the new fail-closed exact-vector contract so the mandatory convention file no longer describes the removed drop-and-renormalize behavior and no longer references reconstruct.active_weights as the renormalization mechanism.
Was this helpful? React with 👍 or 👎 to provide feedback.
| ) or not math.isclose(sum(values), 1.0, abs_tol=1e-9): | ||
| raise ValueError("active lineage weights must be finite, positive, and sum to one") |
There was a problem hiding this comment.
📝 Info: Sum-to-one tolerance matches the weight loader
The new abs_tol=1e-9 sum check could in principle fail-close persisted weights, but load_estimated_channel_weights (backend/app/lineage_ingestion.py:302) already enforces the identical abs_tol=1e-9 sum check before reconstruction, and estimate_channel_weights normalizes by dividing by the total. Loaded and estimated vectors sum to 1 within float epsilon, so the stricter gate adds no new failure path.
Was this helpful? React with 👍 or 👎 to provide feedback.
| expected = set(_CORE_WEIGHT_CHANNELS) | ||
| if getattr(llm, "available", False): | ||
| expected.add("llm") | ||
| if set(weights) != expected: | ||
| raise ValueError("weights must exactly match the active lineage channels") |
There was a problem hiding this comment.
📝 Info: Exact-match change is consistent across all callers
Switching from drop-and-renormalize to exact-match-or-raise could break a caller passing a four-channel vector with an unavailable llm. Every caller derives its channel set from the same available predicate active_weights uses: estimated_weight_channels, analysis_run_start (active_channels gated on _ProviderBoundaryAdjudication.available at backend/app/analysis_run_start.py:77), the demo server, and seed scripts. None passes a set that mismatches its own llm availability.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Contract
Verification
This PR is stacked on #636; a stack merge is not protected-main delivery evidence.