Skip to content

fix(catalog): preserve context inference through provider edits - #7077

Open
lcxhh521 wants to merge 1 commit into
lidge-jun:devfrom
lcxhh521:fix/provider-context-inference-roundtrip
Open

lcxhh521 wants to merge 1 commit into
lidge-jun:devfrom
lcxhh521:fix/provider-context-inference-roundtrip

Conversation

@lcxhh521

@lcxhh521 lcxhh521 commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Follow-up to #7024: preserve inferContextWindow through the provider management API.

  • Include the optional value in GET /api/providers; accept boolean PATCH values and null to clear it. Reject invalid types before applying sibling changes.
  • Preserve the latest live value when a same-name POST omits the field, including a concurrent PATCH while destination validation is awaiting. Explicit submitted values still win.
  • Add eight focused route regressions and document the API contract. The default-off inference policy, built-in provider behavior, and existing context caps are unchanged. No GUI changes.

Verification

Based on merged dev aca5ccfcc209a3b64075336987247db0a20c5615.

  • Red baseline on the pre-fix implementation: PATCH rejected the field; same-name POST lost both existing true and false values.
  • Bun 1.4.3, guarded focused run: management-provider-context-inference, vendor-context, and responses-compaction-http-isolation: 27 passed, 0 failed, 549 assertions.
  • bun run typecheck, bun run structure:check, bun run privacy:scan, and git diff --check: passed.
  • bun test tests/test-layout.test.ts tests/test-layout-tooling.test.ts tests/ci-workflows/file-size-ratchet.test.ts: 28 passed, 0 failed.
  • Docs build: 569 pages; 79,489 internal links checked.
  • The focused run used temporary homes, fail-closed network/subprocess guards, and a synthetic unavailable-Codex-runtime stub for the catalog fixture. API tests exercise the real schema and config loader with an injected fixture writer, rather than the full production save/migration path.
  • Full-suite exception: the broader import-connected graph contains other provider/auth fixtures with potential real external requests; earlier broad execution was stopped by safety review, so it was not safely runnable in this cloud environment. This is not a demonstrated regression in this PR. The eight focused API regressions cover the changed GET/PATCH/POST behavior, strict input rejection, omitted/explicit values, and concurrent PATCH preservation; the production save/migration path remains outside this fixture coverage. Required exact-head CI still needs maintainer execution authorization before merge; results from feat(catalog): infer custom-provider context windows from model metadata #7024 are not validation of this follow-up.
  • Independent source review of this narrow fix found no blocking issue. This is technical review, not maintainer approval. At the readiness check, the branch was 10 commits behind dev 2acdb925229f8812cc9db24bfb1b114bfed35ff2, within the repository's limit.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.

Review readiness checklist

This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:

  • Required local validation passed; commands, results, and any full-suite exception are documented.

  • I pushed my PR to a recent dev commit (at most 10 behind; a maintainer may still ask for the exact tip before merge).

  • I resolved all correct Codex and CodeRabbit findings.

  • My PR is ready for review.

Summary by CodeRabbit

  • New Features
    • Provider management responses now include the configured context-window inference setting.
    • You can set or clear the setting with a PATCH request. When saving a provider with the same name, omitting the setting preserves its latest value; an explicitly submitted boolean takes precedence.
  • Documentation
    • Updated provider configuration and management API references with the setting’s behavior, including its default-off, custom-provider-only availability.

@coderabbitai

coderabbitai Bot commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: lidge-jun/opencodex/.coderabbit.yaml
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 3d8cfb4d-96e6-43a9-8f9d-a4701d2f0764
📥 Commits

Reviewing files that changed from the base of the PR and between aca5ccf and c667570.

📒 Files selected for processing: 9
  • docs-site/src/content/docs/reference/configuration/providers.md
  • docs-site/src/content/docs/reference/management-api.md
  • scripts/test-layout/layout.json
  • src/server/management/provider-overwrite-carry.ts
  • src/server/management/provider-routes.ts
  • structure/catalog-vendor-context.md
  • structure/gui-and-management-api.md
  • tests/fixtures/test-layout-expected.json
  • tests/server/management-provider-context-inference.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

📝 Walkthrough

The provider management API now exposes inferContextWindow in reads, accepts boolean or null PATCH updates, and preserves the latest value during same-name POST saves when the request omits it.

Changes: Provider context inference

Layer / File(s) Summary
Provider reads and PATCH updates
src/server/management/provider-routes.ts:925, src/server/management/provider-overwrite-carry.ts:217-223, tests/server/management-provider-context-inference.test.ts:1-123, scripts/test-layout/layout.json:1209, tests/fixtures/test-layout-expected.json:773, docs-site/src/content/docs/reference/configuration/providers.md:18-24
GET exposes the configured value. PATCH sets it with a boolean, clears it with null, and rejects other types. Tests cover persistence, catalog-cache invalidation, and rejected updates.
Same-name POST preservation
src/server/management/provider-routes.ts:1369-1374, tests/server/management-provider-context-inference.test.ts:125-179, docs-site/src/content/docs/reference/configuration/providers.md:423-426, docs-site/src/content/docs/reference/management-api.md:650-655, structure/catalog-vendor-context.md:14-16, structure/gui-and-management-api.md:278
When POST omits the field, the route preserves its latest live value, including changes made during destination validation. An explicitly submitted value takes precedence. Tests cover stored values, validation, and concurrent PATCH updates. The documentation also describes the preserveResponsesReasoningContent form setting.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant ProviderManagementAPI
  participant DestinationValidator
  participant LiveProviderConfig
  Client->>ProviderManagementAPI: POST provider without inferContextWindow
  ProviderManagementAPI->>DestinationValidator: Validate destination
  Client->>ProviderManagementAPI: PATCH inferContextWindow during validation
  ProviderManagementAPI->>LiveProviderConfig: Apply boolean or clear field
  DestinationValidator-->>ProviderManagementAPI: Validation completes
  ProviderManagementAPI->>LiveProviderConfig: Read latest inferContextWindow
  ProviderManagementAPI-->>Client: Save candidate with latest value
Loading
Merge Risk: ⚪ Minimal · up to c6675

This change keeps the context-inference setting intact when providers are edited through the management API. No merge-blocking risk was identified in the supplied material.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 3 files. (6 skipped: 6… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: preserving context inference through provider edits. It matches the provider API behavior and documented objective.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 3 files. (6 skipped: 6 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added the bug Something isn't working label Oct 13, 2026
@github-actions

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

@github-actions

github-actions Bot commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

✅ READY

  • all PR quality gates passed; the review readiness checklist is complete.

Review readiness checklist

  • ✅ Required local validation passed; commands, results, and any full-suite exception are documented.
  • ✅ I pushed my PR to a recent dev commit (at most 10 behind; a maintainer may still ask for the exact tip before merge).
  • ✅ I resolved all correct Codex and CodeRabbit findings.
  • ✅ My PR is ready for review.

✅ 4/4 boxes ticked.

This pull request is already Ready for Review.
The review-ready label marks this PR as ready; review automation runs independently.
Maintainers: @lidge-jun @Ingwannu

@github-actions
github-actions Bot marked this pull request as ready for review October 13, 2026 19:08

This branch has not been deployed

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

Labels

bug Something isn't working review-ready

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant