Skip to content

fix(compression): persist headroom minRows through full stack - #8111

Closed
rafaumeu wants to merge 6 commits into
cheaper-inference:mainfrom
rafaumeu:fix/headroom-minrows-persist-8056
Closed

rafaumeu wants to merge 6 commits into
cheaper-inference:mainfrom
rafaumeu:fix/headroom-minrows-persist-8056

Conversation

@rafaumeu

Copy link
Copy Markdown
Contributor

Summary

Fixes #8056

The Headroom engine's minRows detail setting silently failed to persist. The UI accepted edits but handleSave() early-returned for headroom because it wasn't in SETTINGS_SUBOBJECT.

Root cause

  1. Frontend: SETTINGS_SUBOBJECT only had aggressive and ultra — handleSave() was a no-op for headroom
  2. Schema: compressionSettingsUpdateSchema had no headroom sub-object — even if the UI POSTed, validation would reject
  3. Runtime: buildStepOptions() didn't inject stored headroom config into stepConfig — SmartCrusher always got DEFAULT_MIN_ROWS (8)

Changes (3 files + 1 test)

**EngineConfigPage.tsx\**:

  • Add headroom: "headroom" to SETTINGS_SUBOBJECT
  • Remove stale comment about structural engines

**compressionConfigSchemas.ts\**:

  • New headroomConfigSchema (minRows int 2-10000, enabled boolean)
  • Register in compressionSettingsUpdateSchema

**strategySelector.ts\**:

  • buildStepOptions() injects options.config.headroom into stepConfig when step.engine === "headroom"

Test plan

5 regression tests verifying all three layers (UI map, schema, runtime injection).

rafaumeu added 5 commits July 22, 2026 00:51
…heaper-inference#8074)

resolveEmbeddingSource() was always returning dimensions=null for
remote sources, even when the model is well-known in the registry
(e.g. nvidia/nv-embedqa-e5-v5 → 1024, openai/text-embedding-3-small
→ 1536). This caused the EmbeddingResolution.dimensions field to be
unusable for callers that need to pre-allocate vector stores or detect
dimension conflicts without performing an actual embed call.

Fix:
- Import getEmbeddingDimension from embeddingRegistry.
- Call getEmbeddingDimension(model) in both remote branches (explicit
  'remote' source and auto-detected remote).
- Use the resolved dimension in the dimensions field and in the
  signature string so the cache-key includes the actual dim.
- Include dim=N in the reason string when dim is known (regression guard).
- Unknown models gracefully fall back to null (no registry entry).

Tests: all 25 in memory-resolve-dimensions-8074 + memory-embedding-resolve
pass; full memory suite 86/86 green.
…-inference#8072)

The /models catalog already exposes effort_tiers for thinking-capable
models, but the Combo Builder path was missing it entirely. Add
effortTiers to ComboBuilderModelOption, propagate through addModelOption
(both create and merge paths), and populate via CANONICAL_EFFORT_VALUES
in all four model loops (synced, builtin, custom, fallback) when
supportsThinking is resolved true.
…cheaper-inference#8059)

Copilot device-code flow stores identity under providerSpecificData.githubEmail
with no top-level email. The dedup logic gated the entire match behind
'if (tokenData.email)', so refreshes with connectionId but no email fell through
to createProviderConnection, creating a duplicate on every refresh.

Fix: match by connectionId first (before the email gate), then fall back to
email-based dedup only when no connectionId match was found.
… paths (cheaper-inference#8081)

Create stripPlaceholderFromContent() helper and apply it across all
response output paths to prevent the internal sentinel
'(prior reasoning summary unavailable)' from leaking into user-visible
assistant content.

Patched paths:
- responseTranslator.ts (non-streaming, both Claude and OpenAI shapes)
- openai-to-claude.ts (streaming text_delta)
- openai-responses.ts (streaming Responses API text)
- jsonToSse.ts (non-streaming JSON to SSE conversion)

The helper uses substring matching (not exact-match) so model-added
prefixes/wrappers are also caught.
- Add 'headroom' to SETTINGS_SUBOBJECT so EngineConfigPage handleSave()
  writes the sub-object instead of early-returning
- Add headroomConfigSchema (minRows: int 2-10000, enabled: boolean) and
  register it in compressionSettingsUpdateSchema
- Inject stored headroom config into buildStepOptions stepConfig so the
  runtime SmartCrusher honors the persisted minRows
- Remove stale comment claiming structural engines have no detail store

Fixes cheaper-inference#8056
@rafaumeu
rafaumeu requested a review from diegosouzapw as a code owner July 22, 2026 11:04
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

The subagent fix (cheaper-inference#8056) handled the write-path (SETTINGS_SUBOBJECT +
buildStepOptions) but the read-path switch in getCompressionSettings had
no case for 'headroom'. The value was written to DB but silently ignored
on read, so minRows never made it back to the UI.

- Add HeadroomConfig interface + DEFAULT_HEADROOM_CONFIG to types
- Add normalizeHeadroomConfig() with floor of 2 rows
- Add case 'headroom' to the switch in getCompressionSettings
@diegosouzapw

Copy link
Copy Markdown
Collaborator

Thanks for this. I'm consolidating the #8056 headroom-minRows persistence on #8058 (RaviTharuma), which lands the same DB read-path fix (normalizeHeadroomConfig + the case "headroom" in getCompressionSettings()) as a single-purpose PR on the release branch, with a dedicated round-trip test. This branch bundles five separate issues onto a stale main base (#8056 + #8072 + #8059 + #8081 + memory-embedding), so I'd rather take the focused version and reconcile your other fixes individually — some are landing (e.g. #8071). Closing in favor of #8058.

@rafaumeu

Copy link
Copy Markdown
Contributor Author

Closing per maintainer feedback. Favoring #8058 (RaviTharuma) — focused single-issue PR. Will re-submit other fixes individually.

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