fix: omit unsupported sampling defaults - #820
Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📝 WalkthroughWalkthrough샘플링 온도의 기본값을 Changes샘플링 온도 전달 정책
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to The change omits unsupported implicit sampling defaults while preserving explicit sampling values across request paths. Remaining issues are limited to documentation, test assertions, and a type annotation; no actionable merge-blocking risk remains after normal checks and review. Sequence Diagram(s)sequenceDiagram
participant RequestScope as 요청 범위
participant ModelClient
participant Provider
RequestScope->>ModelClient: 요청별 temperature 전달
ModelClient->>ModelClient: effective temperature 계산
ModelClient->>Provider: 값이 있으면 temperature 포함
ModelClient->>Provider: 값이 없으면 필드 생략
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
Exact current head |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…faults' into fix/omit-unsupported-sampling-defaults # Conflicts: # contextual_orchestrator/orchestrator.py # tests/test_sampling_defaults.py
…faults' into fix/omit-unsupported-sampling-defaults
Exact-head sampling remediation
Please review and run hosted Checks for |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
@opencode-agent @cwl-noema-review Please review exact head |
|
Current-head diagnosis for contextual-orchestrator PR 820 (a92d524), failed check osv-scan run 32507758842: the base scan and head scan both exited 0, but the required-output step failed because checkout cleanup removed old-results.json before test -s old-results.json. This is the central workflow evidence-retention defect repaired by ContextualWisdomLab/.github PR 1158 at current repair head acbd253df81e06d18ed758de1ce748ad6729faa0. Please rerun after PR 1158 reaches protected main; no manifest suppression or fabricated clean result is appropriate. |
Exact-head gate disposition — WAIT_AND_REMEDIATE
Decision: |
|
@opencode-agent @cwl-noema-review 재검토 요청: exact head 4959e80. implicit sampling 값은 계속 생략하고 explicit request/client 값만 전달하며, 중복 temperature 상태를 삭제했습니다. focused 89 passed, full 1444 passed, changed added line coverage 100%, Ruff correctness/compileall/diff check 통과, 모든 thread resolved입니다. 중앙 OSV workflow #1208 병합 후 실패 check를 재실행합니다. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
Exact-head consumer mapping for LineageWeave. The Azure reasoning-model failure that rejects an implicit temperature is addressed by this PR's omission of unsupported sampling defaults. Current exact head is 4959e80; the source-side sampling tests are reported green, while the remaining hosted blocker is the shared OSV output-file workflow failure. After the central repair reaches protected main, rerun this exact head and obtain independent approval. No provider model selector or local fallback is being added here. |
|
Exact-head audit (2026-08-22): head 4959e80, base e226e11. Hosted osv-scan failed after the scanner exited 0 because the workflow required non-empty old-results.json/new-results.json; opencode-review remains queued. This is an inherited central gate contract issue, not evidence that the sampling-default diff is safe to merge. Formal approvals: 0. Decision: WAIT_AND_REMEDIATE; rerun after the central gate repair. No bypass or forced merge. |
|
The failed |
|
Exact-head RCA for 4959e80, run 32514293613 job 96872308507: the OSV scanner inspected the repository successfully and exited 0 after scanning 76 directories and the lockfile manifests. The required workflow then failed because it still passed deprecated --output=new-results.json and asserted old-results.json/new-results.json, which were not produced. This is the shared ContextualWisdomLab/.github workflow output-file defect, not a source vulnerability or a sampling-default regression. Normal repair is .github PR #1158 at exact head c45a776f9ec3be8b35ee105e966100c80b95c2cc; rerun this exact head after that protected repair. No suppression, fabricated success, or bypass. |
|
Exact head 7b788ca closes the concurrent-request gap behind the Azure sampling failure. HTTP temperature, top_p, penalties, and output-token overrides now live in thread-local request scope instead of mutating shared ModelClient defaults; streamed controls use the same scope. A two-request overlap regression proves explicit temperature 0.2 cannot leak into an omitted request, and nested scopes restore cleanly. Validation: full suite 1446 passed; focused HTTP/sampling suite 28 passed; compileall, diff check, and Ruff correctness checks passed. |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
Exact-head validation for 83d1326 passed: 1446 tests in 531.77 seconds, including 28 focused sampling and judge regressions plus compileall and diff hygiene. The structured judge now uses the same request-scoped output-token contract as chat, stream, and batch paths. |
Exact-head verification refresh
|
|
@opencode-agent Review only exact current head 83d1326 after the central OSV workflow repair lands and the failed check is rerun. Verify omitted sampling controls, explicit-value preservation, request-scope concurrency isolation, and structured/stream/batch parity. Submit a formal Reviews API verdict without changing the branch, self-approving, or bypassing protection. |
|
@opencode-agent Review only exact current head |
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
OpenCode cannot approve yet because required coverage evidence did not pass.
Review outcome
1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
-
Problem: The required coverage-evidence job result was
failure, so OpenCode cannot establish approval sufficiency for this head. -
Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.
-
Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports
successwith required evidence or explicit no-source not-applicable evidence. -
Regression test: Keep the approval branch checking
needs.coverage-evidence.result == successbefore posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present. -
Result: REQUEST_CHANGES
-
Reason: coverage-evidence result was
failure, so required test/docstring evidence was not proven for current head83d1326f9149e84dbe58fec0739efca5da99eead. -
Head SHA:
83d1326f9149e84dbe58fec0739efca5da99eead -
Workflow run: 32554918411
-
Workflow attempt: 1
Coverage evidence
Coverage evidence job did not run or did not publish coverage evidence.
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Changed file (5 files)"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file (5 files)"]
R1 --> V1["required checks"]
Evidence --> S2["Docs: 0123-omit-unsupported-sampling-defaults.md"]
S2 --> I2["operator or user guidance"]
I2 --> R2["Review risk: Docs: 0123-omit-unsupported-sampling-defaults.md"]
R2 --> V2["docs review"]
Evidence --> S3["Test (5 files)"]
S3 --> I3["regression suite"]
I3 --> R3["Review risk: Test (5 files)"]
R3 --> V3["targeted test run"]
OpenCode Review Overview
Pull request overviewOpenCode cannot approve yet because required coverage evidence did not pass. Review outcome1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
Coverage evidenceCoverage Decision
Changed-File Evidence Mapflowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Changed file (5 files)"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file (5 files)"]
R1 --> V1["required checks"]
Evidence --> S2["Docs: 0123-omit-unsupported-sampling-defaults.md"]
S2 --> I2["operator or user guidance"]
I2 --> R2["Review risk: Docs: 0123-omit-unsupported-sampling-defaults.md"]
R2 --> V2["docs review"]
Evidence --> S3["Test (5 files)"]
S3 --> I3["regression suite"]
I3 --> R3["Review risk: Test (5 files)"]
R3 --> V3["targeted test run"]
|
Stale infrastructure-only verdict: central coverage dependency materialization was repaired and merged in ContextualWisdomLab/.github#1210. A fresh independent exact-head review remains required before merge.
There was a problem hiding this comment.
Pull request overview
OpenCode cannot approve yet because required coverage evidence did not pass.
Review outcome
1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
-
Problem: The required coverage-evidence job result was
failure, so OpenCode cannot establish approval sufficiency for this head. -
Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.
-
Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports
successwith required evidence or explicit no-source not-applicable evidence. -
Regression test: Keep the approval branch checking
needs.coverage-evidence.result == successbefore posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present. -
Result: REQUEST_CHANGES
-
Reason: coverage-evidence result was
failure, so required test/docstring evidence was not proven for current head83d1326f9149e84dbe58fec0739efca5da99eead. -
Head SHA:
83d1326f9149e84dbe58fec0739efca5da99eead -
Workflow run: 32562272678
-
Workflow attempt: 1
Coverage evidence
Coverage Decision
- Result: FAIL
- Test evidence: not proven passing
- Docstring evidence: not proven passing when configured
- Failure count: 1
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Changed file (5 files)"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file (5 files)"]
R1 --> V1["required checks"]
Evidence --> S2["Docs: 0123-omit-unsupported-sampling-defaults.md"]
S2 --> I2["operator or user guidance"]
I2 --> R2["Review risk: Docs: 0123-omit-unsupported-sampling-defaults.md"]
R2 --> V2["docs review"]
Evidence --> S3["Test (5 files)"]
S3 --> I3["regression suite"]
I3 --> R3["Review risk: Test (5 files)"]
R3 --> V3["targeted test run"]
|
Exact-head verification for
This removes the coverage sandbox external DNS dependency while retaining the authorization-header and validated-destination assertions. |
Stale exact-head review: its only blocking evidence was the old-head coverage run DNS failure. Head 79363b0 pins provider resolution at the existing validation seam; focused tests (20) and the full suite (1,446) pass. A fresh exact-head review remains required.
opencode-agent
Bot
added
priority: medium
|
Current-head security/check repair: The prior Strix run completed and reported one Medium SSRF path: the HTTP agent-pool create endpoint allowed an authenticated administrator to persist an The fix keeps configurable local runtimes available at trusted process startup but rejects local provider schemes at the remotely mutable HTTP registration boundary, before persistence. Remote HTTPS provider validation, public-address enforcement, and destination pinning remain unchanged. ADR 0124 records the decision with APA 7 OWASP SSRF and NIST SP 800-207 grounding. Verification:
No port-number allowlist, provider-specific bypass, protection bypass, or raw provider fallback was added. |
|
Closing as superseded: main already implements explicit-sampling preservation across chat, stream, and batch paths via |
github-project-automation
Bot
moved this from In Progress
to Done
in naruon Platform Roadmap
1 participant
Summary
Responses API, and provider-probe requests
one shared resolver
requests cannot leak values through shared client defaults
and address-pinning boundary used by runtime provider calls
entries
guidance and the OWASP SSRF Prevention Cheat Sheet (APA 7 in ADRs 0123-0124)
This fixes the observed Azure/LiteLLM rejection of an invented
temperature: 0.2forgpt-5.6-sol. When the caller omits the field, everyprovider path now omits it; an explicit caller value remains explicit and may
still receive the provider's documented capability error. Model discovery,
provider ordering, fallback groups, and reasoning-effort policy are unchanged.
The follow-up security repair resolves Strix's review finding without adding a
provider-specific bypass: configured local registration endpoints must resolve
to loopback addresses and the validated destination is pinned through the HTTP
connection.
Exact-head evidence
9cae444ab01ff8a9b6e1eee928b42bf43c8dc9d5e226e1197bdfc890c9d8e5b9b648c78857d7e4651,447 passed in 530s72 passed100%diff hygiene: passed
Protected-main independent approval remains mandatory. No protection bypass,
force push, provider-specific model exception, raw provider fallback, or secret
materialization is used.