Conversation
Copilot stopped work on behalf of
makr-code due to an error
March 16, 2026 14:30
Copilot stopped work on behalf of
makr-code due to an error
March 16, 2026 16:23
Copilot stopped work on behalf of
makr-code due to an error
March 16, 2026 16:47
Copilot stopped work on behalf of
makr-code due to an error
March 16, 2026 16:59
Copilot stopped work on behalf of
makr-code due to an error
March 16, 2026 17:39
Copilot stopped work on behalf of
makr-code due to an error
March 16, 2026 21:23
Co-authored-by: makr-code <150588092+makr-code@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] [LLM-DEP-123] Implement RocksDB model storage for LLMDeploymentPlugin
[LLM-DEP-123] Implement RocksDB model storage for LLMDeploymentPlugin
Mar 16, 2026
Owner
|
@copilot Review und Code Audit. Wurden alle Aufgaben des issue erfüllt? |
There was a problem hiding this comment.
Pull request overview
This pull request updates the LLM deployment plugin to enable RocksDB/BaseEntity model metadata persistence, add thread-local request context for audit user propagation, and tighten model/source validation, with accompanying tests and a dedicated CI workflow.
Changes:
- Enable
LLMModelStorageinitialization + RocksDB persistence path inLLMDeploymentPlugin. - Add thread-local
RequestContexthelpers and use it to populateaudit.user; reject emptymodel_idearly. - Update
findBestSource()to skip missing “local” sources; add tests and an LLM-focused CI workflow.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
src/llm/llm_deployment_plugin.cpp |
Adds TLS request context, empty model_id rejection, and improved local-source selection; enables RocksDB persistence block. |
include/llm/llm_deployment_plugin.h |
Exposes RequestContext API and includes model storage header. |
tests/llm/test_llm_deployment_plugin.cpp |
Adds tests intended to cover RocksDB init, request context/audit user propagation, empty model_id behavior, and local-source skipping. |
src/llm/FUTURE_ENHANCEMENTS.md |
Marks the RocksDB storage enhancement items as completed. |
.github/workflows/llm-deployment-plugin-rocksdb-storage-ci.yml |
Adds a CI workflow to build and run the relevant LLM deployment/plugin tests. |
You can also share your feedback on Copilot code review. Take the survey.
Comment on lines
+181
to
+187
| static void setRequestContext(const RequestContext& ctx) noexcept; | ||
|
|
||
| /// Clear the authentication context for the calling thread. | ||
| static void clearRequestContext() noexcept; | ||
|
|
||
| /// Return the user_id from the thread-local request context, or @p fallback. | ||
| static std::string currentUserId(const char* fallback = "system") noexcept; |
| std::replace(sanitized.begin(), sanitized.end(), '/', '_'); | ||
| src_path /= sanitized; | ||
| // Also try with .gguf extension if no extension present | ||
| if (!fs::exists(src_path)) { |
Comment on lines
+314
to
+323
| TEST_F(LLMDeploymentPluginTest, StorageInitialisedWhenDbProvided) { | ||
| // When a RocksDBWrapper is injected the plugin must enable model_storage_. | ||
| // We verify this indirectly: deploying a model in OFFLINE mode with an | ||
| // unknown ID must still exercise the constructor path without crashing. | ||
| DeploymentConfig config; | ||
| config.mode = DeploymentMode::OFFLINE; | ||
| config.cache_directory = test_dir_; | ||
| config.use_base_entity_storage = false; // db is null – filesystem-only | ||
| config.enable_audit_log = false; | ||
|
|
Comment on lines
+399
to
+407
| TEST_F(LLMDeploymentPluginTest, FindBestSourceSkipsLocalMissingModel) { | ||
| // Create a "local" source that points to our test_dir_ but does NOT | ||
| // contain the requested model. findBestSource (called internally by | ||
| // downloadModel) should skip that source and fall back to the default | ||
| // Ollama source rather than returning an unusable local entry. | ||
| DeploymentConfig config; | ||
| config.mode = DeploymentMode::OFFLINE; // prevent actual download | ||
| config.cache_directory = test_dir_; | ||
| config.enable_audit_log = false; |
Comment on lines
+54
to
+55
| - `[x]` Uncomment the RocksDB persistence block at line 273; inject `StorageEngine*` into `LLMDeploymentPlugin` constructor. | ||
| - `[x]` Implement `TODO(enhancement)` at line 916: check `model_id` existence before deployment to surface clear errors for unknown model IDs. |
| sudo apt-get update -qq | ||
| sudo apt-get install -y --no-install-recommends \ | ||
| cmake ninja-build g++ \ | ||
| libgtest-dev \ |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
LLMDeploymentPluginoperated in filesystem-only mode: model metadata was never persisted to RocksDB, breaking "list all deployed models" across restarts. Audit entries hardcoded"system"as the user regardless of the authenticated caller.Changes
include/llm/llm_deployment_plugin.h#includewith real#include "llm/llm_model_storage.h"RequestContextstruct +setRequestContext/clearRequestContext/currentUserIdstatic API (thread-local, mirrorsTaskSchedulerpattern)src/llm/llm_deployment_plugin.cppLLMModelStoragewhenuse_base_entity_storage=trueand adbis injectedsaveModelToStoragecall indeployModel(was guarded by/* */sincellm_model_storage.cpp"didn't exist")audit.user = "system"hardcodings withcurrentUserId()deployModelrejects emptymodel_idimmediately with a logged audit failurefindBestSourcevalidates emptymodel_idearly; iterates sources in priority order and skips"local"type entries whose model file is absent — surfaces a clear error instead of silently picking an unusable sourceTests (
tests/llm/test_llm_deployment_plugin.cpp) — 6 new testsStorageInitialisedWhenDbProvidedAuditUserDefaultsToSystem"system"AuditUserPropagatedFromRequestContextRequestContextClearRestoresSystemFallbackclearRequestContextresets stateDeployEmptyModelIdReturnsFalseFindBestSourceSkipsLocalMissingModelOther
llm-deployment-plugin-rocksdb-storage-ci.ymladdedsrc/llm/FUTURE_ENHANCEMENTS.mdcheckboxes marked[x]Type of Change
Testing
📚 Research & Knowledge (wenn applicable)
/docs/research/angelegt?/docs/research/implementation_influence/eingetragen?Relevante Quellen:
Checklist
Original prompt
LLMDeploymentPlugin: RocksDB Model Storage #4011💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.