Skip to content

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

Merged
lidge-jun merged 1 commit into
lidge-jun:devfrom
lcxhh521:fix/provider-context-inference-roundtrip
Oct 15, 2026
Merged

lidge-jun merged 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 provider management edits.

  • Include configured true and false values in GET /api/providers.
  • Accept a boolean or null through PATCH, rejecting other types before saving sibling fields. Clearing restores the default-off behavior.
  • Preserve an omitted value during same-name POST, including destination changes and concurrent PATCH updates/clears while validation awaits. An explicit submitted boolean wins.
  • Add eight route regressions and update the management API, configuration, and structure documentation. Built-in inference policy and context caps retain their existing behavior.

The contributor implementation was reviewed and retained unchanged during maintainer triage; no replacement PR or rewritten contributor history was needed.

Verification

Validated head 01d806fbc3a7436a93615d3350b186cd42094c45, based on dev 7640a942b35044c974e534b84d5a8049f4a8ae9a, with Bun 1.4.3 on macOS on 2026-10-15.

  • bun install: passed without dependency changes.
  • bun run typecheck: passed.
  • bun scripts/test.ts ./tests/server/management-provider-context-inference.test.ts ./tests/codex-integration/vendor-context.test.ts ./tests/responses/responses-compaction-http-isolation.test.ts ./tests/test-layout.test.ts ./tests/test-layout-tooling.test.ts ./tests/ci-workflows/file-size-ratchet.test.ts: 63 passed, 0 failed; 1,202 assertions across 6 files.
  • bun run test:changed: 8,073 passed, 32 skipped, 0 failed; 57,465 assertions across 351 files, compared against the dev merge base above.
  • Regression proof: temporarily restored the two changed source files from origin/dev; all 8 route regressions failed on the original behavior. Restored the exact PR sources and ran OCX_TEST_NO_QUEUE=1 bun scripts/test.ts --parallel=1 ./tests/server/management-provider-context-inference.test.ts: 8 passed, 0 failed; 103 assertions. The tiny red/green runs used one worker with intentional overlap after waiting for the shared test lock; broader verification respected the shared queue. The worktree matches the PR head.
  • bun run structure:check, bun run privacy:scan, and git diff --check: passed.
  • Security review of the management API diff: strict boolean/null PATCH validation precedes persistence; invalid sibling combinations leave memory and fixture disk unchanged. POST validates the complete candidate and reads omitted inference state under the config mutation lock. The diff adds no credential handling, request-body logging, authentication changes, or dependencies.
  • Full-suite resource exception: multiple triage lanes share one 16-core machine, so bun run test was intentionally not run. The changed graph plus explicit source-oracle/layout/ratchet guards and focused catalog/compaction coverage were run instead. Broader Ubuntu coverage remains with CI; macOS/Windows dispatch belongs to the orchestrator.
  • Limits: the route fixture injects its config writer while exercising the real config schema and loader; production save/migration and native service execution are outside that fixture's coverage. GUI was unchanged.
  • Exact-head GitHub workflows were approved for execution during triage. React Doctor and the PR metadata checks have passed; Ubuntu tests, catalog async contracts, and other functional gates are still queued. Pending/skipped checks are not passing evidence. Required checks must finish successfully before merge.

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

  • 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.

@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: d0c6aaab-562b-4048-b043-679c35f42ff4
📥 Commits

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

📒 Files selected for processing: 4
  • docs-site/src/content/docs/reference/management-api.md
  • scripts/test-layout/layout.json
  • structure/gui-and-management-api.md
  • tests/fixtures/test-layout-expected.json

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

📝 Walkthrough

The provider management API 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, src/server/management/provider-overwrite-carry.ts, tests/server/management-provider-context-inference.test.ts, scripts/test-layout/layout.json, tests/fixtures/test-layout-expected.json, docs-site/src/content/docs/reference/configuration/providers.md, docs-site/src/content/docs/reference/management-api.md, structure/catalog-vendor-context.md, structure/gui-and-management-api.md
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. The documentation describes the read, update, and clear behavior.
Same-name POST preservation
src/server/management/provider-routes.ts, tests/server/management-provider-context-inference.test.ts, docs-site/src/content/docs/reference/configuration/providers.md, docs-site/src/content/docs/reference/management-api.md, structure/catalog-vendor-context.md, structure/gui-and-management-api.md
When POST omits the field, the route preserves its latest live value, including changes made during destination validation. An explicitly submitted boolean takes precedence. Tests cover stored values, validation, and concurrent PATCH updates. The documentation also describes the preserveResponsesReasoningContent form setting.

Priority: ➖ Normal

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

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant ProviderRoutes
  participant DestinationValidation
  participant ProviderConfig
  Client->>ProviderRoutes: POST without inferContextWindow
  ProviderRoutes->>DestinationValidation: Validate destination
  Client->>ProviderRoutes: PATCH inferContextWindow during validation
  ProviderRoutes->>ProviderConfig: Apply boolean or clear field
  DestinationValidation-->>ProviderRoutes: Validation completes
  ProviderRoutes->>ProviderConfig: Read latest inferContextWindow
  ProviderRoutes-->>Client: Save candidate with latest value
Loading
Merge Risk: ⚪ Minimal · up to 01d80

Provider management now exposes and updates context-inference settings while preserving omitted values on same-name saves. No concrete merge-blocking risk is established.

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. (4 skipped: 4… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
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.
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, PATCH, and same-name POST changes.
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. (4 skipped: 4 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
Replay the reviewed patch on dev 7640a94.
Validated exact tree daa8631 with focused regressions, typecheck, structure and privacy checks.
No new production behavior is introduced by this refresh.
@lcxhh521
lcxhh521 force-pushed the fix/provider-context-inference-roundtrip branch from c667570 to 01d806f Compare October 15, 2026 03:53
@github-actions
github-actions Bot marked this pull request as draft October 15, 2026 03:54
@github-actions
github-actions Bot marked this pull request as ready for review October 15, 2026 03:55
@lidge-jun
lidge-jun merged commit 65ad37d into lidge-jun:dev Oct 15, 2026
47 of 74 checks passed
lidge-jun pushed a commit that referenced this pull request Oct 15, 2026
Keep both management-api paragraphs: dev's inferContextWindow note (#7077) and this carry's per-model quota note.

Co-authored-by: bi-boo <bi-boo@users.noreply.github.com>
lcxhh521 added a commit to lcxhh521/opencodex that referenced this pull request Oct 15, 2026
Integrate upstream dev 0b13396 with current lidge-jun#6454/lidge-jun#6594, lidge-jun#6381,
lidge-jun#5947, lidge-jun#7120 and maintainer lidge-jun#7147. lidge-jun#7077 is already in upstream.
Use lidge-jun#7147 recovery without restoring the obsolete lidge-jun#7098 prefix gate.

Preserve previous personal dev as first parent and retain its DSH
profile patch and real-home write protection through the newer
upstream implementations. Keep backup/dev-before-integration-20261015.

Validated typecheck, structure, privacy and file-size gates, plus
focused isolated synthetic/local-fixture regression tests. This is
source integration only; no release, installation or deployment.
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.

2 participants