Refactor: Crate Extraction - #8
Open
vinniefalco wants to merge 15 commits into
Open
Conversation
Profile switches re-hashed every sha256-pinned GGUF on every cache hit: ensure_blob verified the cached file unconditionally, and the path-source branch of ensure_model did the same. With local models at 19-29 GB, that hash pass dominated switch time (a live spike measured roughly eight minutes of provisioning before llama-server even started loading weights). A new verified.rs submodule records a marker after any successful hash: the pinned digest, the file size, and the modification time. verify_blob consults the marker first and returns MarkerHit with zero disk reads when all three match; anything missing, stale, corrupt, or mis-pinned falls back to a real hash, which refreshes the marker on success and deletes it on mismatch. URL-source blobs keep the marker beside the file under the existing artifact lock; path sources, which live outside the cache, key a marker under the cache's markers directory by source_cache_key. - Trust tradeoff, documented in the module docs: size+mtime is spoofable by anyone who can write the cache, but the cache root is already operator-trusted, and a marker is only ever written after a real hash match, so the pin still fully guards downloads. Six tests cover first-hash marker write, marker-hit short circuit, content-change re-hash and mismatch, corrupt-marker fallback, post-download marker write, and the path-source second call. Focused: cargo test --locked -p promptforge-gateway local::artifacts (34 passed, 0 failed); fmt and clippy -D warnings clean.
Weight loading and inference in a llama-server child compete with the interactive desktop for CPU and I/O scheduling. The production spawn path now builds its Command through a production_command helper that sets the Win32 BELOW_NORMAL_PRIORITY_CLASS creation flag on Windows, so a profile switch's model load yields to foreground rendering and input handling. The constant is spelled as its raw stable-ABI value to avoid a windows-sys dependency in the main build. Respawns inherit the flag through the same ChildSpawner. Non-Windows builds are a documented no-op; a nice port would need libc and pre_exec unsafe and is deferred. - Test deviation from the plan: the workspace forbids unsafe_code at the unoverridable forbid level, so the planned windows-sys GetPriorityClass probe cannot compile here. The test instead spawns a PowerShell child that self-reports (Get-Process -Id $PID).PriorityClass and asserts BelowNormal - same behavioral coverage, no unsafe, no new dependency, no lockfile change. Focused: cargo test -p promptforge-gateway local::server (24 passed, 0 failed); fmt and clippy -D warnings clean.
Switching to a Gemma-4 profile failed at start-local with "no tool dialect matched the provided evidence". The Gemma-4 chat template uses pipe-wrapped markers (<|tool_call|>, <|tool_response|>, <|turn|>), so the ChatML conjunction in openai_score missed it, and the gemma3 fingerprint missed too: the template has no <start_of_turn> and this server's /props carries no default_generation_settings.model id. Meanwhile the capability evidence that would have settled it - chat_template_caps.supports_tool_calls in /props - was never read; only /v1/models' has_tool_call_capability was probed. fetch_props_evidence now reads the /props chat_template_caps field first, falling back to the /v1/models probe only when it is absent, consistent with the file's props-first precedence. openai_score gains the Gemma-4 conjunction, scoring like the ChatML and Mistral conjunctions, so a tool-capable Gemma-4 template resolves to the native openai dialect even when both capability probes are silent. gemma3_tool_code scoring is unchanged; legacy Gemma-3 evidence still resolves to the emulated dialect, pinned by the pre-existing test. Five new tests cover the marker conjunction, precedence over the gemma fingerprint, absent-vs-false capability parsing, the props capability path, and the unreliable-caps-false fall through. Focused: cargo test --locked -p promptforge-gateway local::dialect (15 passed, 0 failed); fmt and clippy -D warnings clean.
Native compilation inside a running gateway would make ordinary startup depend on developer toolchains, permit long and failure-prone build processes inside the serving lifecycle, and complicate installer security. The change adds one rule to `AGENTS.md`: runtime and serve paths never compile native dependencies or invoke compilers or build tools. Compilation belongs to the Cargo build or packaging process; runtime may only verify, stage, and launch build-produced artifacts.
A marker-write failure after a successful digest match failed verification and download publication, even though the marker only skips a future re-hash. This change adds `write_marker_best_effort`, which wraps `write_marker` and degrades a persistence failure to a `tracing::warn!` log. `verify_blob` and the post-download publish path in `artifacts.rs` now call it. - Confinement stays a hard error: `validate_cache_path` still runs before the marker write, and `LocalError::UnsafeCachePath` and `LocalError::DigestMismatch` remain failures. - New tests `marker_persistence_failure_still_verifies` and `post_download_marker_persistence_failure_still_publishes` use `with_readonly_file` to block the marker path and pin that the operation succeeds while the stale marker stays untouched.
Give built-in tools, web providers, and a future addon host one stable tool vocabulary that does not depend on the parser, the Lua runtime, the executor, or HTTP clients. Move `Tool`, `ToolCatalog`, `ToolId`, `ToolOutput`, `OutputTrust`, `ToolError`, and the contract errors out of `promptforge-core` into the new publishable `promptforge-tools` crate. `promptforge-core` re-exports the vocabulary under `promptforge_core::tools`, so existing paths keep working. - `promptforge-webfetch` now depends on `promptforge-tools` instead of `promptforge-core`; the concrete `WebSearch` tool remains in `promptforge-core`. - `ToolId::from_validated` becomes `#[doc(hidden)] pub` so `promptforge-core` can construct validated identities across the crate boundary, and `NearDuplicateDiagnostic` stays crate-internal to `promptforge-core`. - Because `OutputTrust` is `#[non_exhaustive]` outside its defining crate, `tool_loop.rs` now sends any unknown future variant through `untrusted::wrap` as untrusted output. - The contract tests move with the code into `promptforge-tools`; `promptforge-core` keeps only regression tests that pin re-export identity and dynamic dispatch.
`WebSearch` performs network I/O and owns credentials, deadlines, and response decoding, so it does not belong in the execution core. The concrete provider moves from `crates/promptforge-core/src/tools/web_search.rs` into the new publishable `promptforge-web-search` crate, which keeps the vendor credential behind one review boundary. `promptforge-core` re-exports `WebSearch` under its historical `promptforge_core::tools` path so existing callers keep working. - The new crate takes its `Tool` vocabulary from `promptforge-tools` and carries crate-private `Endpoint` and `Token` types, so it never depends on `promptforge-core` or the gateway; `promptforge-cli`, `promptforge-dev`, and `promptforge-mcp-server` now import `WebSearch` from `promptforge_web_search` directly. - Constructor failures now wrap the endpoint or token error with `ToolError::with_source` instead of flattening it into the message, preserving the underlying cause as the error source. - The HTTP test suite moves with the provider into `crates/promptforge-web-search/src/web_search/tests.rs`; new tests pin source-preserving constructor errors that do not echo embedded credentials, and `reexported_web_search_is_the_provider_type` proves at compile time that the core re-export names the provider type.
Whisper inference is a compute subsystem with worker ownership and its own CUDA dependency, so it moves out of the HTTP server to keep voice CUDA distinct from llama CUDA and to reduce server dependency weight. The `transcribe` module and `segment.rs` move from `promptforge-ws-server` into the new `promptforge-transcribe` crate; the server keeps the voice WebSocket session, route state, and activation, and constructs the engine through the new plain-value `EngineConfig`. - `VoiceEngine::new` now takes `&EngineConfig`; `From<&VoiceConfig> for promptforge_transcribe::EngineConfig` in `config.rs` is the only seam, and an empty `final_model` maps to `None`, which disables the final pass. - The server's GPU feature is renamed to `voice-cuda`, which forwards to `promptforge-transcribe/cuda`; `cuda` remains as a compatibility alias. - `final_pass_absent_for_test` is now gated on the `test-fixtures` feature instead of `cfg(test)`, and `SILENCE_RMS` and `rms` become crate-private. - The moved engine code carries no new tests; the only added test is `voice_config_maps_into_engine_config`, which pins the `VoiceConfig` mapping including the empty-`final_model` case.
Windowing and the Windows WebView2 bridge are platform infrastructure, not application lifecycle orchestration, so they move out of the `promptforge-ws` binary into an unpublished crate. The new crate owns `window.rs`, `file_drop.rs`, the icon assets, and the tao/wry event loop behind one public `run` entry point. `promptforge-ws` keeps configuration discovery, gateway start, the health wait, and shutdown, and opens the window through `promptforge_desktop_shell::run`. - `promptforge-desktop-shell` mirrors the workspace lint set with `unsafe_code` lowered to deny, which `file_drop.rs` alone opts out of; `promptforge-ws` returns to `workspace = true` lints. - `window::run` widens from `pub(crate)` to `pub` as the crate's single documented entry point. - CI excludes `promptforge-desktop-shell` from the default workspace clippy, test, doctest, doc, and MSRV jobs and adds it to the workshop clippy and test jobs. - `window.rs` and `file_drop.rs` move without behavior changes; the only added test, `run_is_the_single_narrow_entry_point` in `tests/it/main.rs`, pins the `run` signature, and the moved windowing code gains no other new tests.
On Windows the gateway hard-codes a Vulkan llama.cpp archive even on NVIDIA systems. The new `llama-cuda` feature compiles the pinned `third_party/llama.cpp` submodule during the Cargo build into a host-native CUDA `llama-server` and embeds the bundle in the gateway binary. The unpublished `promptforge-gateway-build` crate runs the pipeline behind the `Probe` command seam; the gateway `build.rs` calls `build()` and includes the generated `llama_cuda_bundle` module. - Build logic lives in `promptforge-gateway-build` (`publish = false`), not in the gateway library, and `crates/promptforge-gateway/AGENTS.md` limits runtime code to verifying, staging, and launching build-produced bundles. - The submodule is pinned to `PINNED_COMMIT` (tag `b10082`); `submodule::verify` reads HEAD without invoking git and fails the build on absence or drift, naming both commits. - The build applies only to native Windows x86-64: `require_native` rejects cross-compilation, every other target returns `built: false`, and `workshop-cuda` now implies `llama-cuda`. - The pipeline requires CUDA Toolkit >= `MIN_TOOLKIT` (12.8), detects visible GPUs through `nvidia-smi`, compiles only the `llama-server` target for the normalized `CMAKE_CUDA_ARCHITECTURES` list, and smoke-checks `llama-server --list-devices` for a CUDA device. - `dumpbin /dependents` accounting keeps Windows system and CUDA Toolkit DLLs external under `LINKAGE_POLICY` `static-project-external-cuda`; an unknown import absent from the bundle fails the build. - All output stays under `OUT_DIR`: a canonical `llama-cuda-manifest.json` (`BUNDLE_FORMAT_VERSION` 1, SHA-256 per file) and a generated `llama_cuda_bundle.rs` embedding `MANIFEST` and `FILES` with `include_str!` and `include_bytes!`. - The `llama_cuda_bundle` module is dormant under `#[expect(dead_code)]`; no runtime code consumes the embedded bundle yet. Tests use synthetic trees and `FakeProbe`; no test invokes real CMake or the network.
A `llama-cuda` Windows x86-64 build now verifies and publishes its embedded CUDA bundle into the operator cache instead of downloading the Vulkan archive. The new `cuda_bundle` module decodes the embedded manifest through a narrow runtime-side schema, validates the payload, checks the declared external CUDA Toolkit DLLs, and stages the files through the shared lock, private staging directory, tree digest, install marker, and atomic rename. `provision_llama_server` returns the new `ProvisionedServer`, whose `path_prefix` `production_command` prepends to the child `PATH`. - `build.rs` emits the `llama_cuda_embedded` cfg from Cargo target environment variables, so runtime code gates on one name and a cross-compile cannot claim an embedded bundle it did not produce. - The runtime decodes the manifest into its own `RuntimeManifest` and reports failures through `BundleError` wrapped by `LocalError::CudaBundle`; it never depends on the build-support crate. - The toolkit runtime directory resolves from `CUDA_PATH_V13_3`-style versioned variables with `CUDA_PATH` as fallback, and each external DLL must exist in that `bin` directory or in `System32`. - Payload validation completes before the cache is consulted, so tampered embedded bytes fail even when a valid installation exists; a valid matching installation returns without restaging. - `child_path_with_prefix` sets the prefixed `PATH` only on the child environment; the process environment is never mutated, and an empty prefix leaves the inherited `PATH` untouched. - `stage_embedded`, the consumer of `crate::llama_cuda_bundle`, is not covered by tests; the tests exercise `stage_bundle` with synthetic payloads.
The bundle build failed on Visual Studio generators because `parse_cache` required `CMAKE_CXX_COMPILER` and `CMAKE_CXX_COMPILER_VERSION` entries in `CMakeCache.txt`, which those generators never write: the toolset fixes the compiler, so the cache carries only the generator. `parse_generator` now reads the generator alone, and the new `parse_compiler_cmake` and `compiler_cmake_path` recover the compiler path and version from `CMakeFiles/<version>/CMakeCXXCompiler.cmake`. - `build_with` in `bundle.rs` now assembles `CacheIdentity` itself from the `parse_generator` and `parse_compiler_cmake` results; `cmake.rs` no longer owns cache-wide parsing. - `compiler_cmake_path` sorts the `CMakeFiles` subdirectories and returns the first that contains `CMakeCXXCompiler.cmake`; a missing file fails the build with an error that names `CMakeCXXCompiler.cmake`. - The new test `missing_compiler_identity_fails_the_build` pins that failure, and the synthetic host now writes a `CMakeFiles/4.4.2/CMakeCXXCompiler.cmake` fixture while the manifest test asserts the `msvc` path and version.
When `llama_cuda_embedded` is set, provisioning stages an embedded bundle and never reads archives, so `require_executable`, four `LocalError` variants, and the `LocalError` import in `cuda_bundle.rs` had no users. The dead code and the unused import fail lint runs that deny warnings. This change gates each item behind `cfg(not(llama_cuda_embedded))`, removes the import, and points the doc links at the full path `crate::local::error::LocalError`. - The variants `Archive`, `MissingExecutable`, and `DuplicateExecutable` use `cfg(any(not(llama_cuda_embedded), test))` so the archive tests still compile; `UnsupportedPlatform` has no test use and is fully gated out. - In `artifacts.rs`, the `llama_cuda_embedded` branch drops its `return` for a tail expression, because the trailing `not(llama_cuda_embedded)` block compiles away and leaves a needless `return`. - No test assertions change; the only test edit adds the `LocalError` import to `cuda_bundle/tests.rs`.
Chat local models need an explicit MTP drafter and multimodal projector declared next to the main model, so those artifacts stay reproducible and pinned without repository-name heuristics. A new `companion` module in `promptforge-gateway-config` parses and validates a `[local_model.speculative]` sub-table and a `[local_model.multimodal_projector]` sub-table into `SpeculativeConfig` and `MultimodalProjectorConfig`, exposed as optional `speculative` and `multimodal_projector` fields on `LocalModelConfig` and re-exported through `lib.rs`. - The shared `validate_artifact_source` gate in `companion.rs` now holds the artifact source rules: non-empty, `https` or local path, remote sources pinned by `sha256`, and a 64-character lowercase hex check. `validate.rs` calls it for the main model source in place of its inline checks, and `validate_http_url` is now `pub(super)` to permit that reuse. - `DraftTokenMax` bounds `draft_max` to `1..=16` and `SpeculationType` supports only `draft-mtp`, so an unknown speculation type fails at deserialize time. Both companion types use `deny_unknown_fields`, and `validate_kind_scope` rejects them on any non-chat model kind. - The companions are parsed, validated, and re-exported only; no code outside `promptforge-gateway-config` consumes them.
Declared companions must reach every launch and respawn as the same verified paths and arguments. `LocalRuntime::start` now calls `provision_companions` before `ServerGuard::start`, which resolves the speculative drafter and multimodal projector through `store.ensure_model` and stores the owned results in `LaunchOptions` as `speculative` and `multimodal_projector`. `server_args` emits `--spec-draft-model`, `--spec-type draft-mtp`, `--spec-draft-n-max`, and `--mmproj` when those fields are set. - Each companion resolves through `ensure_model` under its own source identity and pin, so the drafter and projector share the main model's integrity and cache behavior. - The new `SpeculativeLaunch` struct owns the resolved drafter path and draft maximum inside `LaunchOptions`, so a respawn re-emits the exact verified artifact without re-resolving external state. - The companion flag spellings are pinned to the bundled server (`third_party/llama.cpp` @ `fb0e6b6`, `common/arg.cpp`); the legacy `--draft` and `--draft-max` flags were removed at that pin. - A provisioning failure returns `LocalError` before the child spawns, so a bad companion never becomes a spawned-then-failing server. - A model without companions leaves `LaunchOptions` untouched, preserving the exact command line from before companions existed.
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
1 participant
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.
bunch of stuff.. check commit messages