Skip to content

fix(llm): forward consolidation temperature - #3511

Open
lusoris wants to merge 1 commit into
vectorize-io:mainfrom
lusoris:fix/consolidation-temperature-forwarding
Open

fix(llm): forward consolidation temperature#3511
lusoris wants to merge 1 commit into
vectorize-io:mainfrom
lusoris:fix/consolidation-temperature-forwarding

Conversation

@lusoris

@lusoris lusoris commented Aug 15, 2026

Copy link
Copy Markdown

Summary

Forward llm_temperature_consolidation to both observation-consolidation LLM calls:

  • the batch synthesis call (scope="consolidation")
  • the semantic dedup adjudication call (scope="consolidation_dedup")

The configuration field and its environment resolution were added by #2469 for #2459. The current call sites do not pass the resolved value, so HINDSIGHT_API_LLM_TEMPERATURE_CONSOLIDATION has no effect on these two operations. This also means the global none setting cannot omit temperature consistently across all consolidation calls.

Changes

  • Pass the resolved consolidation temperature to both LLM call sites.
  • Add a focused batch-call regression test with a non-default temperature.
  • Assert that dedup adjudication receives the configured default.
  • Complete the minimal test config objects used by existing consolidation tests.

Verification

  • pytest tests/test_consolidation_retry_budget.py tests/test_consolidation_dedup.py -q — 52 passed.
  • pytest tests/test_consolidation.py::TestBuildResponseModel -q — 8 passed.
  • pytest tests/test_llm_temperature_pipeline.py -q — 3 passed.
  • ./scripts/hooks/lint.sh — passed.
  • Repository pre-commit hooks — passed. The existing vulture and knip findings remained advisory and do not involve this diff.

Risk

Low. This uses the existing resolved configuration and provider behavior. The default remains 0.0. A configured None continues through LLMConfig.call() so providers can omit the parameter for models that reject explicit temperatures.

Related

AI assistance

OpenAI Codex with the gpt-5 model, high reasoning, tool use, and code execution. The client does not expose the context-window size.

@koriyoshi2041

Copy link
Copy Markdown
Contributor

Checked exact head 467e0987 locally. The consolidation wiring and focused regressions behave as intended: 60 tests passed across retry-budget, dedup, temperature-pipeline, and response-model coverage.

The remaining 3 temperature-pipeline cases stopped during fixture setup before exercising this patch because the local environment points at a SOCKS proxy without socksio; this is an environment dependency error, not an assertion failure. In particular, the new 0.65 batch assertion and the dedup temperature assertion both passed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants