Skip to content

feat: removes from_memory handling from API handlers to make them only serve data from DB - #4286

Closed
roroghost17 wants to merge 1 commit into
06-10-feat_expose_skills_repository_navigation_and_access_statefrom
06-08-feat_removes_from_memory_handling_from_api_handlers_to_make_them_only_serve_data_from_db
Closed

feat: removes from_memory handling from API handlers to make them only serve data from DB#4286
roroghost17 wants to merge 1 commit into
06-10-feat_expose_skills_repository_navigation_and_access_statefrom
06-08-feat_removes_from_memory_handling_from_api_handlers_to_make_them_only_serve_data_from_db

Conversation

@roroghost17

@roroghost17 roroghost17 commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Summary

Removes the from_memory query parameter support from the GET /api/governance/virtual-keys and GET /api/governance/virtual-keys/{vk_id} endpoints. Previously, callers could bypass the config store and read virtual key data directly from in-memory governance state. These code paths have been removed so both endpoints exclusively use the config store.

Changes

  • Removed the from_memory branch from getVirtualKeys, which previously read from GetGovernanceData, hydrated virtual keys with model config governance rules, and returned a sorted slice.
  • Removed the from_memory branch from getVirtualKey, which previously looked up a single virtual key by ID from in-memory state and applied model config governance before responding.
  • Both endpoints now always delegate to the config store, simplifying the handler logic and eliminating a parallel read path.

Type of change

  • Bug fix
  • Feature
  • Refactor
  • Documentation
  • Chore/CI

Affected areas

  • Core (Go)
  • Transports (HTTP)
  • Providers/Integrations
  • Plugins
  • UI (React)
  • Docs

How to test

go test ./...

Verify that GET /api/governance/virtual-keys?from_memory=true and GET /api/governance/virtual-keys/{vk_id}?from_memory=true behave identically to requests without the parameter — the query argument should now be silently ignored and the config store response returned in both cases.

Screenshots/Recordings

N/A

Breaking changes

  • Yes
  • No

Callers explicitly passing from_memory=true will no longer receive in-memory data. The parameter is now ignored and the config store is always queried. Any client relying on the in-memory read path must be updated to use the standard config store response.

Related issues

N/A

Security considerations

Removing the in-memory read path reduces the surface area for stale or inconsistent data being served to callers. No auth, secrets, or PII implications.

Checklist

  • I read docs/contributing/README.md and followed the guidelines
  • I added/updated tests where appropriate
  • I updated documentation where needed
  • I verified builds succeed (Go and UI)
  • I verified the CI pipeline passes locally if applicable

Summary by CodeRabbit

  • Improvements
    • Enhanced governance handling for virtual keys with improved data hydration and consistency.
    • Added support for additional filtering parameters to provide more flexible control over virtual key retrieval.

@coderabbitai

coderabbitai Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 57a75c55-70b4-4ddf-af76-14e49254ae6d

📥 Commits

Reviewing files that changed from the base of the PR and between 92886e0 and b2d98ee.

📒 Files selected for processing (1)
  • transports/bifrost-http/handlers/governance.go
💤 Files with no reviewable changes (1)
  • transports/bifrost-http/handlers/governance.go

📝 Walkthrough

Walkthrough

The PR consolidates virtual key retrieval from in-memory governance lookups to DB-backed storage. The getVirtualKeys list handler now uses paginated database queries with filtering parameters, while getVirtualKey single-item handler always loads from the database. Both handlers apply governance hydration before responding.

Changes

Virtual Key Handler DB Consolidation

Layer / File(s) Summary
Virtual keys list handler consolidation
transports/bifrost-http/handlers/governance.go
getVirtualKeys removes in-memory path, consolidates to DB-backed pagination and filtering with extended query parameters (export, exclusion flags), hydrates governance for returned slice via reverse-mapped VK-scoped model configs.
Single virtual key handler consolidation
transports/bifrost-http/handlers/governance.go
getVirtualKey removes in-memory path, always loads from DB, applies hydrateVKGovernance, returns 404 on missing VK or 500 on error.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

  • maximhq/bifrost#3903: Both PRs remove the from_memory query-path in transports/bifrost-http/handlers/governance.go for governance virtual-key retrieval and shift handlers to DB-backed ConfigStore logic.

Poem

🐰 From memory's fleeting cache we spring,
To databases where truths take wing!
No more in-memory shadows dance,
Hydration blooms with governance's glance,
DB-backed and steadfast now, we prance! ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and specifically describes the main change: removing from_memory handling to make handlers only serve data from DB.
Description check ✅ Passed The description is well-structured, covers all major template sections, clearly explains the changes, breaking changes, and testing approach, with only non-critical checklist items incomplete.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 06-08-feat_removes_from_memory_handling_from_api_handlers_to_make_them_only_serve_data_from_db

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

@CLAassistant

CLAassistant commented Jun 11, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

roroghost17 commented Jun 11, 2026

Copy link
Copy Markdown
Contributor Author

@greptile-apps

greptile-apps Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Confidence Score: 5/5

The change is a clean, intentional removal of a parallel read path with no correctness concerns in the remaining code.

The deletion is surgical — two if fromMemory { ... } blocks are removed and the rest of each handler is left untouched. Shared helpers used elsewhere are unaffected. No imports become dangling. The DB-only path that remains was already well-tested by the existing paginated and non-paginated branches. The only risk is callers that pass from_memory=true now silently receive store data instead; this is documented as a breaking change in the PR description.

No files require special attention — the single changed file is straightforward.

Important Files Changed

Filename Overview
transports/bifrost-http/handlers/governance.go Removes the from_memory branch from getVirtualKeys and getVirtualKey; both handlers now exclusively use the config store. Logic is clean, no dead imports introduced, and shared helpers (buildVKModelConfigIndex, applyVKGovernanceFromModelConfigs) remain used by other callers.

Reviews (1): Last reviewed commit: "feat: removes from_memory handling from ..." | Re-trigger Greptile

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