Repository navigation
Conversation
Every backend used to pass its own confidence through validation, so Laya's normalised entropy, Jev's margin and Cua's max p were not comparable. Compute contract confidence in validation.py from the validated probabilities; keep each backend's number as backend_confidence on the answer, in Decision.raw, and on tool/browser ticks. Closes #89.
Birfy
left a comment
There was a problem hiding this comment.
Review (own PR — commenting, not approving)
Reviewed the full diff against #89. Looks correct and ready for a human approval once CI is green.
What works
- Contract confidence is computed once in
validation.validate_choice/validate_noulasmax_k p_k/max(p, 1-p), so backends can no longer silently disagree on scale. - Backend-native values land on
Choice/Noul.backend_confidence,Decision.raw["backend_confidence"], and tool/browser ticks when present — matches the provenance acceptance criterion. choice_faultscorrectly stops requiring a backendconfidencein [0, 1]; out-of-range / missing natives are dropped rather than faulted.- Docs + CHANGELOG call out that historical
laya/jev/omnijevconf=values are not comparable with the new contract — important for anyone reading old runs. - Tests cover the Laya-like entropy≠peak case, OmniJev Jev-margin as
backend_confidence, RandomModel uniform → contract 0.5, and the DecisionModelContract peak-probability invariant. No change to which option is chosen.
Nits (non-blocking)
- Browser ticks only carry
backend_confidencefrom theoperationhead; other heads remain underDecision.raw["backend_confidence"]. That matches the previousMove.confidenceshape — fine, just worth knowing when reading replays. validate_answersoverwrites any pre-existingreply.raw["backend_confidence"]key. Unlikely today; if a backend ever puts that key inrawitself, rename or nest.
Cannot approve my own PR. Once CI is green on this head, this is merge-ready pending human approval.
|
Exact-head CI follow-up for 3125780: the full/core jobs stop at Ruff 0.16.8 formatting, before their test/type/build/smoke stages. The two rejected assertions are tests/test_decision_models_base.py:66 and tests/test_decision_models_laya.py:125. Minimal fix: run I reproduced the formatting failure and checked the local formatting-only patch independently. Whole-tree Ruff format/lint, ty, offline lock check, sdist/wheel build and CLI/MCP smoke pass. Python 3.12 reused core/dev environment: 1007 passed, 130 skipped, 202 subtests passed; focused tests: 75 passed, 3 skipped. This is not fresh 3.11/3.13 CI or real browser/model validation. No branch was pushed. Please apply the formatting change to this head and rerun the exact new commit's CI before treating it as ready. Existing failed job: https://github.com/ThinkFlowLab/system1-agents/actions/runs/39248864853/job/118047414645 |
…and surface every head's on browser ticks
|
Pushed 3734ffc: |
Required human contributor answers
Answer every question before implementation and PR submission, including drafts.
Reuse explicit, still-applicable human answers with a traceable source reference;
ask the human for anything missing and wait. An agent must not invent answers.
Blank fields, placeholders, or an agent-written rationale do not satisfy this gate.
Share only information and source links appropriate for this PR's audience.
What outcome are you trying to achieve?
Answer: One documented meaning for
Choice.confidence/Noul.confidenceon every backend, soconf=columns, MCPdecide, and any future confidence gate compare the same scale (issue [Decisions] Makeconfidencemean the same thing on every backend (Laya reports normalized entropy, others max p or Jev's margin) #89; Danyang authorized filling intent from issue context for scheduled coding runs).What is the highest-leverage variable for that outcome?
Answer: Where confidence is assigned — in
validation.pyfrom validated probabilities, not passed through from each backend's native formula.How will this work move that variable rather than create busywork?
Answer: Recompute as
max_k p_k/max(p, 1-p)in the single validation path; retain each backend's number asbackend_confidenceso provenance is not lost. No change to which option is chosen.Who is the customer or user, and what do they actually need?
Answer: Operators and eval authors reading run summaries / probe / MCP decide output; they need confidence comparable across
laya,jev,omnijev, andcuawithout silent scale mismatches (arXiv:2610.02267).After answering the four questions: is a PR needed at all to achieve this outcome?
Human decision and rationale (or explicit source reference): Yes — issue #89 proposes one PR with the contract change, provenance field, docs, CHANGELOG, and offline tests. Scheduled coding run implements directly per Danyang's Oct 13 guidance.
For a bug fix: link actual observed failure evidence.
Evidence: Not a live thresholding bug (nothing gates on confidence yet). Scale mismatch is documented in #89 and Fast Models, Slow Evidence (arXiv:2610.02267): Laya
confidencediffers frommax_k p_kby ~0.299 on average.Why
Every backend returned a
confidencethat meant something different (Laya: normalised entropy; Jev/OmniJev: normalised margin; Cua: max p). Anything that thresholds or comparesconf=across backends would silently break. Closes #89.How
validation.validate_choice/validate_noulalways set contract confidence from probabilities (max_k p_k/max(p, 1-p)).Choice/Noul.backend_confidence,Decision.raw["backend_confidence"], and tool/browser tick records when present.docs/decision-models.md), CHANGELOG, DecisionModelContract tests, and unit tests updated.Files to open first:
s1a/decision_models/validation.py,s1a/decision_models/types.py,tests/decision_model_contract.py.What
Choice.confidence= peak probability;Noul.confidence=max(p, 1-p).backend_confidenceon answers, raw, and ticks.laya/jev/omnijevconf=values are not comparable with new ones.Verification
uv run ruff format --check/ruff check/ty checkon touched decision-model filespytest -q— 1015 passed, 121 skippedscripts/smoke.sh— okDemo / evidence
backend_confidence).issue-89-unify-confidenceon ThinkFlowLab/system1-agents; no Omni worker change.Closes #89