Skip to content

refactor(code-index): use immutable configuration snapshots - #1815

Open
WebMad wants to merge 1 commit into
Zoo-Code-Org:mainfrom
WebMad:enhancement/1788-immutable-code-index-config
Open

WebMad wants to merge 1 commit into
Zoo-Code-Org:mainfrom
WebMad:enhancement/1788-immutable-code-index-config

Conversation

@WebMad

@WebMad WebMad commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor

Related GitHub Issue

Closes #1788 — Model code-index settings as immutable configuration.
Parent issue: #1592.

Issue approval is not confirmed by the available labels or discussion; maintainer confirmation is still needed.

Description

  • Replace independently mutable configuration fields with a typed immutable snapshot, built completely before publication.
  • Reuse the existing configuration model with readonly fields, narrow provider options, and runtime freezing of snapshots and nested options.
  • Extract provider settings readers, model dimension parsing, and provider resolution into focused methods.
  • Simplify readiness validation and restart comparison while preserving existing defaults and restart rules.
  • Reuse the public configuration projection for load results, including the maximum search result count.
  • Adapt the vector-store tests introduced on main to replace configuration objects rather than mutate readonly properties.

Test Procedure

From the backend workspace, run:

cd src
npx vitest run services/code-index
pnpm exec tsc --noEmit

For focused configuration coverage:

npx vitest run services/code-index/__tests__/config-manager.spec.ts --coverage --coverage.include='services/code-index/config-manager.ts'

Validation:

  • Code-index subsystem after merging current main: 698 tests passed across 30 files.
  • Configuration-manager suite: 119 tests passed; measured local line, statement, branch, and function coverage reached 100%.
  • Tests cover frozen provider options, snapshot isolation, failed snapshot construction, model dimension parsing and warnings, empty Bedrock region and recovery, incomplete defensive snapshots, and the unknown-provider fallback.
  • Workspace lint and type-check hooks passed. ESLint suppression counts are unchanged.
  • Latest CI compile, Linux and Windows unit tests, mutation-diff, and Codecov patch checks passed. E2E and the review gate were still pending when this description was updated.

Pre-Submission Checklist

  • Issue Linked: [ENHANCEMENT] Model code-index settings as immutable configuration #1788 is linked above; explicit issue approval still requires maintainer confirmation.
  • Scope: Changes are focused on immutable code-index configuration and its compatibility tests.
  • Self-Review: Implementation and test diffs have been reviewed, including compatibility with current main.
  • Testing: New and updated focused tests cover the changes; the code-index suite passes.
  • Visual Snapshot (UI changes only): Not applicable; no UI changes.
  • Documentation Impact: Considered; no user-facing behavior or configuration format changes.
  • Contribution Guidelines: Author confirmation of reading and agreement is required; not asserted on the author's behalf.

Visual Snapshots

Not applicable. This is an internal configuration refactor with no visual changes.

Videos (interaction / animation only)

Not applicable. No new interaction or animation.

Documentation Updates

  • No user-facing documentation updates are required. Existing settings and behavior are preserved.

Additional Notes

  • No new dependencies or persisted setting changes.
  • Concurrent load ordering and model dimension validation semantics are unchanged.
  • Main was merged to reproduce the CI merge revision and fix readonly assignments in newly introduced vector-store tests.

Get in Touch

Contact the PR author through this GitHub pull request.

@coderabbitai

coderabbitai Bot commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Important

Review skipped

Auto reviews are limited based on label configuration.

🏷️ Required labels (at least one) (1)
  • coderabbit-review-active

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository: Zoo-Code-Org/Zoo-Code/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 9161ae97-4f5d-4362-842c-780c106d2ff5

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: Zoo-Code-Org/Zoo-Code/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: d26e7e4b-23e3-425e-b15c-5040bba80820

📥 Commits

Reviewing files that changed from the base of the PR and between 38a0cf6 and e48946a.

📒 Files selected for processing (2)
  • src/services/code-index/__tests__/config-manager.spec.ts
  • src/services/code-index/vector-store/__tests__/vector-store-factory.spec.ts

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

📜 Recent review details
⏰ Context from checks skipped due to timeout. (1)
  • GitHub Check: mutation-diff
🧰 Additional context used
📓 Path-based instructions (5)
Check persistence and lifecycle invariants: awaited atomic writes, rollback or explicit partial-failure behavior, cross-window state consistency, stale listeners/watchers, cancellation, idempotency, and safe restart/resume without lost or d...

⚙️ CodeRabbit configuration file

Files:

  • src/services/code-index/__tests__/config-manager.spec.ts
  • src/services/code-index/vector-store/__tests__/vector-store-factory.spec.ts
Require regression coverage at the lowest valid harness with behavior-focused assertions, including relevant negative, error, false/unset, and boundary cases.

⚙️ CodeRabbit configuration file

Files:

  • src/services/code-index/__tests__/config-manager.spec.ts
  • src/services/code-index/vector-store/__tests__/vector-store-factory.spec.ts
Check strict typing and exhaustive behavior across normal, boundary, error, cancellation, retry, and compatibility paths.

⚙️ CodeRabbit configuration file

Files:

  • src/services/code-index/__tests__/config-manager.spec.ts
  • src/services/code-index/vector-store/__tests__/vector-store-factory.spec.ts
Verify extension/webview contracts, cancellation and error propagation, VS Code lifecycle correctness, and behavior under retries and partial failure.

⚙️ CodeRabbit configuration file

Files:

  • src/services/code-index/__tests__/config-manager.spec.ts
  • src/services/code-index/vector-store/__tests__/vector-store-factory.spec.ts
Act as an adversarial second-opinion reviewer.

⚙️ CodeRabbit configuration file

Files:

  • src/services/code-index/__tests__/config-manager.spec.ts
  • src/services/code-index/vector-store/__tests__/vector-store-factory.spec.ts

📝 Summary

Summary by CodeRabbit

  • Bug Fixes

    • Failed configuration reloads leave the current configuration available.
    • Configuration changes are checked safely when settings or credentials are incomplete.
    • Restoring a valid Bedrock region is recognized as requiring a restart.
  • Improvements

    • Configuration updates provide immutable snapshots, and restart requirements reflect relevant setting changes.
    • Model-dimension settings and default search-result limits are handled during configuration loading.

Walkthrough

The code index configuration manager now stores configuration in immutable snapshots. It builds and publishes snapshots during loading, uses snapshot values for readiness and restart decisions, and returns frozen configuration objects. The configuration types and tests reflect these changes.

Changes

Code index configuration snapshots

Layer / File(s) Summary
Immutable snapshot contract
src/services/code-index/interfaces/config.ts
Configuration and previous-state properties are readonly. CodeIndexConfigSnapshot adds the enabled setting, and a helper freezes snapshot objects and their object-valued properties.
Snapshot construction and loading
src/services/code-index/config-manager.ts, src/services/code-index/__tests__/config-manager.spec.ts
The manager builds snapshots from settings and secrets, then publishes them during configuration loading. Tests cover context proxy access, dimension parsing, default search results, frozen configurations, reloads, and failed snapshot builds.
Snapshot-based checks and getters
src/services/code-index/config-manager.ts, src/services/code-index/__tests__/config-manager.spec.ts, src/services/code-index/vector-store/__tests__/vector-store-factory.spec.ts
Readiness, restart decisions, and configuration getters use snapshot values. Tests cover incomplete snapshots, connection values, Bedrock region changes, and invalid provider fallback. Vector-store tests use copied configuration objects instead of mutating them.

Priority: ⬇️ Low

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

Change: Refactor

Architecture Summary

Architecture risk: 🔵 Low · up to e4894

The change affects 1 system.

Changed systems: src

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — src (service) was modified; 4 changed files map to changed impact.

Before / after behavior

  • observed — Modified behavior in src/services/code-index/config-manager.ts: The configuration imports now use CodebaseIndexConfig, CodeIndexConfigSnapshot, and freezeCodeIndexConfigSnapshot; the direct ApiHandlerOptions import is removed.
  • observed — Modified behavior in src/services/code-index/config-manager.ts: The manager replaces its individual configuration fields with a snapshot and stores the context proxy in the constructor. Construction now reads the current configuration into that snapshot rather than invoking the removed mutating loader.
  • observed — Modified behavior in src/services/code-index/config-manager.ts: The former _loadAndSetConfiguration entry point is replaced by _readConfiguration, which begins assembling configuration from persisted settings and defaults without updating manager state.
  • observed — Modified behavior in src/services/code-index/config-manager.ts: _readConfiguration builds and freezes a complete snapshot, including secrets and provider options. New readers return provider-specific options only when their required values are present; provider resolution falls back to OpenAI for unrecognized values, and model-dimension parsing accepts positive numeric values or warns and returns undefined.
🚥 Pre-merge checks | ✅ 8
✅ Passed checks (8 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.
Regression Evidence ✅ Passed No regression-evidence failure found. The changed configuration behavior has focused unit coverage in src/services/code-index/__tests__/config-manager.spec.ts: model-dimension unset, valid, and inva…
Security Boundaries ✅ Passed No changed path meets the security failure condition. src/services/code-index/config-manager.ts still reads secrets through ContextProxy.getSecret() and passes them to the existing internal config…
Persistence Integrity ✅ Passed No changed persistence write path exists. The refactor only reads codebaseIndexConfig, reads secrets, and awaits contextProxy.refreshSecrets() before publishing an in-memory snapshot. `ContextProx…
Lifecycle Resource Cleanup ✅ Passed No concrete changed lifecycle path can leak a resource or duplicate work. The PR changes CodeIndexConfigManager to read, freeze, and publish configuration snapshots, and `freezeCodeIndexConfigSnapsh…
Title check ✅ Passed The title clearly and concisely describes the primary change: replacing mutable code-index configuration with immutable snapshots.
Description check ✅ Passed The description follows the required template and documents the issue, implementation, testing, checklist status, documentation impact, and additional notes. It explicitly identifies that issue approv…
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

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 commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor

Review status

Thanks for contributing. This comment tracks the review sequence and the next action.

Current step: Awaiting fresh human maintainer or CODEOWNER approval.

Automated review is complete for the latest commit but does not replace human approval.

Review-state labels are managed by this workflow; do not edit them manually.

@codecov

codecov Bot commented Sep 26, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@github-actions github-actions Bot added coderabbit-review-active Required CI passed; CodeRabbit review is active awaiting-coderabbit Waiting for CodeRabbit to approve the latest commit labels Sep 26, 2026
coderabbitai[bot]
coderabbitai Bot previously approved these changes Sep 26, 2026
@github-actions github-actions Bot added awaiting-maintainer CodeRabbit approved; waiting for a human maintainer and removed coderabbit-review-active Required CI passed; CodeRabbit review is active awaiting-coderabbit Waiting for CodeRabbit to approve the latest commit labels Sep 26, 2026
@github-actions github-actions Bot added coderabbit-review-active Required CI passed; CodeRabbit review is active awaiting-coderabbit Waiting for CodeRabbit to approve the latest commit and removed awaiting-maintainer CodeRabbit approved; waiting for a human maintainer labels Sep 28, 2026
@github-actions github-actions Bot added awaiting-maintainer CodeRabbit approved; waiting for a human maintainer and removed coderabbit-review-active Required CI passed; CodeRabbit review is active awaiting-coderabbit Waiting for CodeRabbit to approve the latest commit labels Sep 28, 2026
@WebMad

WebMad commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor Author

Correction: This comment was posted on the wrong PR due to a context mix-up. The reported Persistence Integrity review finding concerns PR #1821, not #1815. The comparison below was performed only for #1815 and does not establish whether the finding is a regression in #1821. No dismissal of the #1821 finding should be inferred from this comment.

Persistence Integrity: verified as a pre-existing defect, not a regression in this PR

We compared the actual PR base d351a155e38abb34040b0a867fb29c8270c634b0 with PR head e48946ab5eff175fc0dbde1256830ded8fcd4c0c (before the history-only squash).

The committed versions of processors/file-watcher.ts, processors/scanner.ts, vector-store/qdrant-client.ts, and interfaces/vector-store.ts under src/services/code-index/ have identical Git blobs on both revisions. This PR changes configuration snapshots and their tests, plus readonly compatibility in the vector-store factory tests; it does not change the persistence paths identified by the warning.

We also ran identical temporary tests in isolated worktrees for both revisions, modeling stored points in memory:

Scenario Base PR head
Watcher: embedding fails after old points are deleted Old points lost Old points lost
Scanner: embedding fails after old points are deleted Old points lost Old points lost
Scanner: parser returns no indexable blocks Cache hash advances without a corresponding vector-store update Same behavior

All three desired-preservation tests failed identically on both revisions. Characterization assertions checking the actual faulty behavior passed on both. Git blame traces the problematic ordering back to 61e122dc34d4e1944f54701ff2cfc4a1c5bea54a (2025-05-23), predating this PR.

The persistence concern is valid, but it is an existing indexing defect rather than a regression introduced by #1815. Please treat its remediation as separate follow-up work rather than expanding the immutable-configuration scope of #1788. A local repair draft was explored, but it is not included in this PR and we are not claiming the persistence defect is fixed.

Limitations: these tests used an in-memory model of the vector store, not a live Qdrant server; crash safety, ambiguous network failures, and concurrent writers were not evaluated.

@WebMad
WebMad force-pushed the enhancement/1788-immutable-code-index-config branch from e48946a to ca63d3f Compare September 28, 2026 12:29
@github-actions github-actions Bot added awaiting-maintainer CodeRabbit approved; waiting for a human maintainer and removed awaiting-maintainer CodeRabbit approved; waiting for a human maintainer labels Sep 28, 2026

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

awaiting-maintainer CodeRabbit approved; waiting for a human maintainer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[ENHANCEMENT] Model code-index settings as immutable configuration

1 participant