Harden ggml/LLM integration paths and remove simulation-backed LLM fallbacks - #6089
Conversation
Co-authored-by: makr-code <150588092+makr-code@users.noreply.github.com>
Co-authored-by: makr-code <150588092+makr-code@users.noreply.github.com>
Co-authored-by: makr-code <150588092+makr-code@users.noreply.github.com>
Co-authored-by: makr-code <150588092+makr-code@users.noreply.github.com>
Co-authored-by: makr-code <150588092+makr-code@users.noreply.github.com>
Co-authored-by: makr-code <150588092+makr-code@users.noreply.github.com>
Co-authored-by: makr-code <150588092+makr-code@users.noreply.github.com>
Co-authored-by: makr-code <150588092+makr-code@users.noreply.github.com>
Co-authored-by: makr-code <150588092+makr-code@users.noreply.github.com>
Co-authored-by: makr-code <150588092+makr-code@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
There are correctness/safety issues in the new/modified hardening code paths (notably ggml buffer copying assumptions and overly strict Postgres bound-parameter control-character rejection) that should be addressed before merge.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR hardens several production execution paths by removing simulation/heuristic fallbacks across LLM speculative decoding, distributed training coordination, validator stress testing, shard RPC routing, and storage restore/ggml bridging, with accompanying test and roadmap/status updates to reflect the new fail-closed behavior.
Changes:
- Harden storage + ggml integration by registering
GGML_TYPE_TTonce duringEmbeddedLLMstartup and mapping TT tensors into real ggml allocations when aggml_context*is provided. - Remove simulation/heuristic fallbacks: speculative remote draft text no longer degrades into byte-modulo token IDs; distributed training and validator stress test paths now fail-closed when required transports/engines are absent.
- Expand/adjust tests and update module and root roadmap/status documents to match the newly enforced runtime contracts.
File summaries
| File | Description |
|---|---|
| tests/test_tensor_phase3.cpp | Updates TARGRetrieval entropy test expectations to match exact entropy path. |
| tests/tensor/test_tensor_phase3.cpp | Same entropy expectation updates for tensor test suite variant. |
| tests/test_speculative_draft_fn_bridge.cpp | Adds SPEC-TL-05 coverage for tokenizer bridge overriding heuristic in local draft path. |
| tests/legacy/speculative/test_speculative_draft_fn_bridge.cpp | Mirrors SPEC-TL-05 coverage in legacy suite. |
| tests/test_shard_rpc_grpc.cpp | Switches unit tests to explicit inproc:// endpoints for in-process routing. |
| tests/legacy/shard/test_shard_rpc_grpc.cpp | Mirrors explicit inproc:// endpoints for legacy shard RPC tests. |
| tests/test_production_validator.cpp | Adds fail-closed coverage when stress test runs without an inference engine. |
| tests/legacy/production/test_production_validator.cpp | Mirrors validator fail-closed stress test coverage in legacy suite. |
| tests/test_new_aql_functions.cpp | Extends AQL function tests for new ethics/process-mining behaviors. |
| tests/test_distributed_txn_api_handler.cpp | Asserts explicit error status when shard endpoints are missing (fail-closed read-only path). |
| src/utils/input_validator.cpp | Introduces validateJsonSchema() and routes critical paths to schema validation (fail-closed). |
| include/utils/input_validator.h | Adds validateJsonSchema() declaration and keeps validateJsonStub() alias for compatibility. |
| src/server/http_server.cpp | Migrates selected routes from validateJsonStub() to validateJsonSchema(). |
| src/server/postgres_session.cpp | Tightens prepared-statement literal binding with typed parsing and placeholder-safe replacement. |
| src/storage/ggml_tensor_bridge.cpp | Uses ggml_context* in map path and copies decompressed TT data into ggml tensor when available. |
| src/storage/backup_manager.cpp | Makes decompress/decrypt restore paths fail-closed when required dependencies are absent. |
| src/storage/ROADMAP.md | Updates storage roadmap status to reflect fail-closed restore and ggml TT wiring progress. |
| src/sharding/shard_rpc_client.cpp | Makes in-process routing explicit (inproc:// / loopback://) rather than implicit loopback detection. |
| src/sharding/distributed_transaction.cpp | Validates shard endpoints and fails closed when endpoints are missing/placeholder. |
| src/sharding/cloud_sdk_integration.cpp | Hardens cloud callback initialization by validating fixed provider arguments and required config. |
| src/sharding/cloud_backup.cpp | Attempts SDK callback bootstrap when callbacks are missing and fails closed if still unavailable. |
| src/sharding/ROADMAP.md | Updates sharding roadmap to reflect callback bootstrap + endpoint validation hardening. |
| src/server/ROADMAP.md | Notes remaining server runtime gaps and marks request-validation schema hardening delivered. |
| src/rag/targ_retrieval.cpp | Switches default entropy computation to full-vocabulary softmax entropy (exact). |
| include/rag/targ_retrieval.h | Updates API docs to match built-in full-vocabulary entropy behavior. |
| src/rag/ROADMAP.md | Updates RAG roadmap status to reflect exact entropy behavior. |
| src/llm/inference_engine_enhanced.cpp | Adds tokenizer-bridge reuse (llama-backed) and removes remote byte-modulo fallback by retrying local draft. |
| src/llm/llama_wrapper.cpp | Adds tokenizeForBridge() to expose llama.cpp tokenizer for bridge usage. |
| include/llm/llama_wrapper.h | Declares and documents tokenizeForBridge() bridge API. |
| src/llm/embedded_llm.cpp | Registers ggml TT type once during embedded LLM startup under THEMIS_ENABLE_GGML_BRIDGE. |
| src/llm/distributed_training_coordinator.cpp | Removes simulated gradients/health behavior; marks shards unavailable and fails closed without transport. |
| src/llm/production_validator.cpp | Requires attached inference engine for stress tests; removes synthetic fallback responses. |
| src/llm/ROADMAP.md | Updates LLM roadmap to reflect closed simulation-backed paths and remaining residual fallback debt. |
| src/llm/QUICK_STATUS_BOARD.txt | Marks quick board as historical snapshot superseded by roadmap. |
| src/query/functions/process_mining_functions.cpp | Replaces process-mining stub behaviors with context/scanner-backed implementations. |
| src/query/functions/ethics_functions.cpp | Replaces ethics stub behaviors with collection/graph/vector-backed execution over FunctionContext. |
| src/query/ROADMAP.md | Updates query roadmap to reflect delivered ethics + process-mining closures. |
| src/MODULE_GAP_ANALYSIS_WAVE2.md | Updates wave gap analysis to reflect revalidation and corrected prioritization. |
| ROADMAP.md | Updates root roadmap “Last Updated” date and refreshes module status classification text. |
Review details
- Files reviewed: 39/39 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| "(Subsequent occurrences for this schema are suppressed.)", | ||
| schema_name, schema_dir_, schema_name); | ||
| } | ||
|
|
||
| } |
There was a problem hiding this comment.
Fixed in the latest commit: the warning message now reads InputValidator::validateJsonSchema instead of validateJsonStub.
There was a problem hiding this comment.
Updated in c97b478: the missing-schema warning now correctly references InputValidator::validateJsonSchema.
| #ifdef THEMIS_HAS_GGML | ||
| if (handle.impl_->real_ggml_tensor && handle.impl_->real_ggml_tensor->data && | ||
| !handle.impl_->fake_tensor.data.empty()) { | ||
| std::memcpy(handle.impl_->real_ggml_tensor->data, | ||
| handle.impl_->fake_tensor.data.data(), | ||
| handle.impl_->fake_tensor.data.size() * sizeof(float)); | ||
| } | ||
| #endif |
There was a problem hiding this comment.
Fixed in the latest commit. Before memcpy, the code now validates that:
- The tensor type is
GGML_TYPE_F32 - The allocated capacity (
ne[0] * sizeof(float)) is at least as large as the data to copy
If either check fails, the memcpy is skipped, an error is logged, and the handle is returned invalid.
There was a problem hiding this comment.
Updated in c97b478: added type/capacity guards before ggml memcpy so incompatible allocator outputs are rejected instead of copied.
| bool containsControlCharacters(const std::string& input) { | ||
| return std::any_of(input.begin(), input.end(), [](unsigned char c) { | ||
| return c == '\0' || std::iscntrl(c); | ||
| }); | ||
| } |
There was a problem hiding this comment.
Fixed in the latest commit. containsControlCharacters now exempts \t, \n, and \r from the control-character check, so valid bound text parameters containing tabs/newlines/carriage-returns are accepted.
There was a problem hiding this comment.
Updated in c97b478: control-character validation now allows tab/newline/carriage-return for bound text parameters while still rejecting unsafe control bytes.
Co-authored-by: makr-code <150588092+makr-code@users.noreply.github.com>
Co-authored-by: makr-code <150588092+makr-code@users.noreply.github.com>
Co-authored-by: makr-code <150588092+makr-code@users.noreply.github.com>
Co-authored-by: makr-code <150588092+makr-code@users.noreply.github.com>
|
Target: pr Doxygen Governance Gate
Changed source files
Blocking structural findings
Doxygen warnings
source_workflow: Gate: PR Doxygen Governance |
✅ Wave C Policy Gate: Edition & License ValidationResult: ✅ Edition & License Validation PASSED Checks
|
✅ Wave C Policy Gate: Community Fail-Closed ValidationResult: ✅ Community Fail-Closed Validation PASSED Checks
|
✅ Wave C Policy Gate: Hash & SBOM IntegrityResult: ✅ Hash & SBOM Integrity PASSED Checks
|
✅ Wave C Policy Gate: Private Plugin Boundary EnforcementResult: ✅ Private Plugin Boundary Enforcement PASSED Checks
|
Co-authored-by: makr-code <150588092+makr-code@users.noreply.github.com>
This PR advances the next storage/LLM block by wiring the ggml TT type registration into the embedded LLM startup path and tightening LLM runtime behavior where production flows still degraded into heuristics or simulation-backed execution. It also updates the source-of-truth module status docs to match the new runtime state.
Storage / ggml bootstrap
GGML_TYPE_TTonce duringEmbeddedLLMstartup underTHEMIS_ENABLE_GGML_BRIDGEstd::call_onceso TT registration is process-wide and deterministicSpeculative decode fallback cleanup
inference_engine_enhanced.cpp, stop converting remote speculative draft text into byte-modulo token IDs when no tokenizer bridge is availableDistributed training hardening
distributed_training_coordinator.cpp, remove fabricated gradient/health behavior when noShardRoutertransport existsProduction validator hardening
production_validator.cpp, require a real inference engine for stress runsFocused test coverage
Docs / source-validated status
src/llm/ROADMAP.mdto reflect the closed speculative and fail-closed validator/training slicessrc/storage/ROADMAP.mdandsrc/MODULE_GAP_ANALYSIS_WAVE2.mdso storage/LLM gap tracking matches the current source stateExample of the new fail-closed contract in the validator path: