-
Notifications
You must be signed in to change notification settings - Fork 6
packing default modelfiles + taking sys prompt from modelfile #76
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 3 commits
f2cb8fa
8cd9294
0d62a91
ab8f516
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,12 +1,13 @@ | ||||||
| #!/usr/bin/env bash | ||||||
| set -euo pipefail | ||||||
|
|
||||||
| ENV="prod" # prod is another env, try taking it from github env | ||||||
| ENV="dev" # prod is another env, try taking it from github env | ||||||
| REPO="tilesprivacy/tiles" | ||||||
| # VERSION="${TILES_VERSION:-latest}" | ||||||
| VERSION="0.4.0-rc.1" | ||||||
| INSTALL_DIR="$HOME/.local/bin" # CLI install location | ||||||
| SERVER_DIR="$HOME/.local/share/tiles/server" # Python server folder | ||||||
| SERVER_DIR="$HOME/.local/lib/tiles/server" # Python server folder | ||||||
| MODELFILE_DIR="$HOME/.local/lib/tiles/modelfiles" # Python server folder | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Misleading comment on MODELFILE_DIR. The comment says "Python server folder" but this is the modelfiles directory. Appears to be a copy-paste error from line 9. Suggested fix-MODELFILE_DIR="$HOME/.local/lib/tiles/modelfiles" # Python server folder
+MODELFILE_DIR="$HOME/.local/lib/tiles/modelfiles" # Modelfiles folder📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||
| TMPDIR="$(mktemp -d)" | ||||||
| OS=$(uname -s | tr '[:upper:]' '[:lower:]') | ||||||
| ARCH=$(uname -m) | ||||||
|
|
@@ -34,6 +35,13 @@ log "📦 Installing tiles binary to ${INSTALL_DIR}..." | |||||
| mkdir -p "${INSTALL_DIR}" | ||||||
| install -m 755 "${TMPDIR}/tiles" "${INSTALL_DIR}/tiles" | ||||||
|
|
||||||
| log "Unpacking libs ..." | ||||||
| rm -rf "${MODELFILE_DIR}" | ||||||
|
|
||||||
| mkdir -p "${MODELFILE_DIR}" | ||||||
|
|
||||||
| cp -r "${TMPDIR}/modelfiles"/* "${MODELFILE_DIR}/" | ||||||
|
|
||||||
| log "📦 Installing Python server to ${SERVER_DIR}..." | ||||||
| rm -rf "${SERVER_DIR}" | ||||||
|
|
||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -59,7 +59,7 @@ class ModelInfo(BaseModel): | |||||||||||||||||||
| class StartRequest(BaseModel): | ||||||||||||||||||||
| model: str | ||||||||||||||||||||
| memory_path: str | ||||||||||||||||||||
|
|
||||||||||||||||||||
| system_prompt: str | ||||||||||||||||||||
|
Comment on lines
59
to
+62
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: # First, let's find and read the server/schemas.py file
fd -type f -name "schemas.py" | grep -E "server|schemas"Repository: tilesprivacy/tiles Length of output: 180 🏁 Script executed: # Also search for dependency files to identify Pydantic version
fd -type f -name "requirements*.txt" -o -name "setup.py" -o -name "pyproject.toml" -o -name "setup.cfg" -o -name "poetry.lock" -o -name "Pipfile"Repository: tilesprivacy/tiles Length of output: 232 🏁 Script executed: # Search for any pydantic-related imports to understand version usage
rg "from pydantic import|import pydantic" -A 2Repository: tilesprivacy/tiles Length of output: 328 🏁 Script executed: # Read the server/schemas.py file to see current state
cat -n server/schemas.py | head -80Repository: tilesprivacy/tiles Length of output: 2088 🏁 Script executed: # Search for pydantic version in common dependency files
find . -maxdepth 2 -type f \( -name "requirements*.txt" -o -name "setup.py" -o -name "pyproject.toml" -o -name "setup.cfg" \) -exec cat {} \;Repository: tilesprivacy/tiles Length of output: 461 🏁 Script executed: # Also check for environment specs or lock files
find . -maxdepth 2 -type f \( -name "poetry.lock" -o -name "Pipfile" -o -name "Pipfile.lock" \) -exec head -50 {} \;Repository: tilesprivacy/tiles Length of output: 44 🏁 Script executed: # Search for how system_prompt is used throughout the codebase
rg "system_prompt" -B 2 -A 2Repository: tilesprivacy/tiles Length of output: 4837 🏁 Script executed: # Check what version of FastAPI is being used (which implies Pydantic version)
rg "fastapi|pydantic" pyproject.toml setup.py requirements.txt 2>/dev/null | head -20Repository: tilesprivacy/tiles Length of output: 44 🏁 Script executed: # Look for any validators or validators decorator on StartRequest
ast-grep --pattern 'class StartRequest { $$$ }'Repository: tilesprivacy/tiles Length of output: 44 Enforce non-empty The field currently accepts empty strings, which would result in blank system messages. The Rust implementation explicitly rejects empty system prompts, and the Python schema should enforce this constraint consistently. 🔧 Proposed fix class StartRequest(BaseModel):
model: str
memory_path: str
- system_prompt: str
+ system_prompt: str = Field(..., min_length=1)📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||
|
|
||||||||||||||||||||
| class downloadRequest(BaseModel): | ||||||||||||||||||||
| model: str | ||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,8 @@ | ||
| { | ||
| "lock_input_hash": "sha256:182c606e20dd957344cc3adc54391f47f4b6dd80b4481ddf219392a7aad6e0ce", | ||
| "lock_version": 1, | ||
| "locked_at": "2026-01-21T09:13:58.607286+00:00", | ||
| "locked_at": "2026-01-22T05:41:48.443112+00:00", | ||
| "other_inputs_hash": "sha256:63b3c2cfe2ec414938e81dace7aac779c7b902bae681618cd8827e9f16880985", | ||
| "requirements_hash": "sha256:41b3e6ec3cd37289edeb1c134ce836c0dfa7843d7dd3dc28a1b46880d77bf029", | ||
| "version_inputs_hash": "sha256:53726e1053a34cced52a7d0c9b2aa679dad94259b51681758674ae4320bbb7a4" | ||
| "requirements_hash": "sha256:a08c15387b6f199fe37fad0855c14ffde941d1c0b49f94fa1ed48a9464fab9a6", | ||
| "version_inputs_hash": "sha256:58db986b7cd72eeded675f7c9afd8138fe024fb51451131b5562922bbde3cf43" | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| [package] | ||
| name = "tilekit" | ||
| version = "0.1.0" | ||
| version = "0.2.0" | ||
| edition = "2024" | ||
|
|
||
| [dependencies] | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -8,7 +8,7 @@ use tiles::{core::health, runtime::RunArgs}; | |||||||||||||||||||
| pub use tilekit::optimize::optimize; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| pub async fn run(runtime: &Runtime, run_args: RunArgs) { | ||||||||||||||||||||
| runtime.run(run_args).await; | ||||||||||||||||||||
| let _ = runtime.run(run_args).await; | ||||||||||||||||||||
|
Comment on lines
10
to
+11
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: # First, let's check the file and the specific lines
cat -n tiles/src/commands/mod.rs | head -20Repository: tilesprivacy/tiles Length of output: 767 🏁 Script executed: # Find the Runtime type definition and run method signature
fd --type=rs | xargs grep -l "impl Runtime" | head -5Repository: tilesprivacy/tiles Length of output: 231 🏁 Script executed: # Search for all runtime.run call sites as mentioned in the review
rg -nP '\bruntime\.run\s*\(.*\)\.await' --type=rsRepository: tilesprivacy/tiles Length of output: 88 🏁 Script executed: # Also check for just runtime.run without .await in case it's not async
rg -nP '\bruntime\.run\s*\(' --type=rs -A 2Repository: tilesprivacy/tiles Length of output: 88 🏁 Script executed: # Find Runtime type definition
find . -name "*.rs" -type f | xargs grep -l "impl Runtime" | head -5Repository: tilesprivacy/tiles Length of output: 88 🏁 Script executed: # Search for runtime.run call sites without using invalid --type flag
rg 'runtime\.run\s*\(' -A 2 --color=neverRepository: tilesprivacy/tiles Length of output: 434 🏁 Script executed: # Look for Runtime::run method signature in the codebase
rg 'fn run\(' -B 3 -A 3 --color=never | head -50Repository: tilesprivacy/tiles Length of output: 1603 Don't swallow runtime errors. Line 11 discards the 🔧 Proposed fix pub async fn run(runtime: &Runtime, run_args: RunArgs) {
- let _ = runtime.run(run_args).await;
+ if let Err(err) = runtime.run(run_args).await {
+ eprintln!("Runtime failed: {err}");
+ std::process::exit(1);
+ }
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| pub fn set_memory(path: &str) { | ||||||||||||||||||||
|
|
||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,7 @@ | ||
| use crate::runtime::RunArgs; | ||
| use crate::utils::config::{ | ||
| create_default_memory_folder, get_config_dir, get_default_memory_path, get_memory_path, | ||
| get_server_dir, set_memory_path, | ||
| create_default_memory_folder, get_config_dir, get_default_memory_path, get_lib_dir, | ||
| get_memory_path, set_memory_path, | ||
| }; | ||
| use crate::utils::hf_model_downloader::*; | ||
| use anyhow::{Context, Result}; | ||
|
|
@@ -17,6 +17,7 @@ use rustyline::{Config, Editor, Helper}; | |
| use serde_json::{Value, json}; | ||
| use std::fs; | ||
| use std::fs::File; | ||
| use std::path::PathBuf; | ||
| use std::process::Stdio; | ||
| use std::time::Duration; | ||
| use std::{io, process::Command}; | ||
|
|
@@ -42,26 +43,29 @@ impl MLXRuntime { | |
| MLXRuntime {} | ||
| } | ||
|
|
||
| pub async fn run(&self, run_args: super::RunArgs) { | ||
| const DEFAULT_MODELFILE: &str = "FROM driaforall/mem-agent-mlx-4bit"; | ||
| //Parse modelfile | ||
| pub async fn run(&self, run_args: super::RunArgs) -> Result<()> { | ||
| let default_modelfile_path = get_default_modelfile()?; | ||
| let default_modelfile = | ||
| tilekit::modelfile::parse_from_file(default_modelfile_path.to_str().unwrap()).unwrap(); | ||
| let modelfile_parse_result = if let Some(modelfile_str) = &run_args.modelfile_path { | ||
| tilekit::modelfile::parse_from_file(modelfile_str.as_str()) | ||
| } else { | ||
| tilekit::modelfile::parse(DEFAULT_MODELFILE) | ||
| Err("NOT PROVIDED".to_string()) | ||
| }; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Avoid using error strings for control flow; handle parsing failures properly. Several issues in this block:
Suggested refactor using Option- let default_modelfile =
- tilekit::modelfile::parse_from_file(default_modelfile_path.to_str().unwrap()).unwrap();
- let modelfile_parse_result = if let Some(modelfile_str) = &run_args.modelfile_path {
- tilekit::modelfile::parse_from_file(modelfile_str.as_str())
- } else {
- Err("NOT PROVIDED".to_string())
- };
-
- let modelfile = match modelfile_parse_result {
- Ok(mf) => mf,
- Err(err) if err == "NOT PROVIDED" => default_modelfile.clone(),
- Err(_err) => {
- println!("Invalid Modelfile");
- return Ok(());
- }
- };
+ let default_modelfile_str = default_modelfile_path
+ .to_str()
+ .context("Default modelfile path is not valid UTF-8")?;
+ let default_modelfile = tilekit::modelfile::parse_from_file(default_modelfile_str)
+ .map_err(|e| anyhow::anyhow!("Failed to parse default modelfile: {}", e))?;
+
+ let modelfile = match &run_args.modelfile_path {
+ Some(modelfile_str) => {
+ match tilekit::modelfile::parse_from_file(modelfile_str.as_str()) {
+ Ok(mf) => mf,
+ Err(err) => {
+ return Err(anyhow::anyhow!("Invalid Modelfile: {}", err));
+ }
+ }
+ }
+ None => default_modelfile.clone(),
+ };🤖 Prompt for AI Agents |
||
|
|
||
| let modelfile = match modelfile_parse_result { | ||
| Ok(mf) => mf, | ||
| Err(err) if err == "NOT PROVIDED" => default_modelfile.clone(), | ||
| Err(_err) => { | ||
| println!("Invalid Modelfile"); | ||
| return; | ||
| return Ok(()); | ||
| } | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| }; | ||
|
|
||
| let _res = run_model_with_server(self, modelfile, &run_args) | ||
| let _res = run_model_with_server(self, modelfile, default_modelfile, &run_args) | ||
| .await | ||
| .inspect_err(|e| eprintln!("Failed to run the model due to {e}")); | ||
| Ok(()) | ||
| } | ||
|
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||
|
|
||
| #[allow(clippy::zombie_processes)] | ||
|
|
@@ -76,10 +80,10 @@ impl MLXRuntime { | |
| } | ||
|
|
||
| let config_dir = get_config_dir()?; | ||
| let mut server_dir = get_server_dir()?; | ||
| let mut server_dir = get_lib_dir()?; | ||
| let pid_file = config_dir.join("server.pid"); | ||
| fs::create_dir_all(&config_dir).context("Failed to create config directory")?; | ||
|
|
||
| server_dir = server_dir.join("server"); | ||
| let stdout_log = File::create(config_dir.join("server.out.log"))?; | ||
| let stderr_log = File::create(config_dir.join("server.err.log"))?; | ||
| let server_path = server_dir.join("stack_export_prod/app-server/bin/python"); | ||
|
|
@@ -257,6 +261,7 @@ fn show_help(model_name: &str) { | |
| async fn run_model_with_server( | ||
| mlx_runtime: &MLXRuntime, | ||
| modelfile: Modelfile, | ||
| default_modelfile: Modelfile, | ||
| run_args: &RunArgs, | ||
| ) -> Result<()> { | ||
| if !cfg!(debug_assertions) { | ||
|
|
@@ -268,7 +273,7 @@ async fn run_model_with_server( | |
| // loading the model from mem-agent via daemon server | ||
| let memory_path = get_or_set_memory_path().context("Setting/Retrieving memory_path failed")?; | ||
| let modelname = modelfile.from.as_ref().unwrap(); | ||
| match load_model(modelname, &memory_path).await { | ||
| match load_model(&modelfile, &default_modelfile, &memory_path).await { | ||
| Ok(_) => start_repl(mlx_runtime, modelname, run_args).await, | ||
| Err(err) => return Err(anyhow::anyhow!(err)), | ||
| } | ||
|
|
@@ -426,11 +431,17 @@ pub async fn ping() -> Result<(), String> { | |
| } | ||
| } | ||
|
|
||
| async fn load_model(model_name: &str, memory_path: &str) -> Result<()> { | ||
| async fn load_model( | ||
| modelfile: &Modelfile, | ||
| default_modelfile: &Modelfile, | ||
| memory_path: &str, | ||
| ) -> Result<()> { | ||
| let client = Client::new(); | ||
| let model_name = modelfile.from.clone().unwrap(); | ||
| let body = json!({ | ||
| "model": model_name, | ||
| "memory_path": memory_path | ||
| "memory_path": memory_path, | ||
| "system_prompt": modelfile.system.clone().unwrap_or(default_modelfile.system.clone().unwrap()) | ||
|
madclaws marked this conversation as resolved.
|
||
| }); | ||
|
|
||
| let res = client | ||
|
|
@@ -442,7 +453,7 @@ async fn load_model(model_name: &str, memory_path: &str) -> Result<()> { | |
| StatusCode::OK => Ok(()), | ||
| StatusCode::NOT_FOUND => { | ||
| println!("Downloading {}\n", model_name); | ||
| match pull_model(model_name).await { | ||
| match pull_model(&model_name).await { | ||
| Ok(_) => { | ||
| println!("\nDownloading completed \n"); | ||
| Ok(()) | ||
|
|
@@ -549,3 +560,8 @@ async fn wait_until_server_is_up() { | |
| } | ||
| } | ||
| } | ||
|
|
||
| fn get_default_modelfile() -> Result<PathBuf> { | ||
| let path = get_lib_dir()?.join("modelfiles/mem-agent"); | ||
| Ok(path) | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -60,17 +60,17 @@ pub fn create_default_memory_folder() -> Result<PathBuf> { | |||||||||||||||||||
| Ok(memory_path) | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| pub fn get_server_dir() -> Result<PathBuf> { | ||||||||||||||||||||
| pub fn get_lib_dir() -> Result<PathBuf> { | ||||||||||||||||||||
| if cfg!(debug_assertions) { | ||||||||||||||||||||
| let base_dir = env::current_dir().context("Failed to fetch CURRENT_DIR")?; | ||||||||||||||||||||
| Ok(base_dir.join("server")) | ||||||||||||||||||||
| Ok(base_dir) | ||||||||||||||||||||
| } else { | ||||||||||||||||||||
| let home_dir = env::home_dir().context("Failed to fetch $HOME")?; | ||||||||||||||||||||
| let data_dir = match env::var("XDG_DATA_HOME") { | ||||||||||||||||||||
| Ok(val) => PathBuf::from(val), | ||||||||||||||||||||
| Err(_err) => home_dir.join(".local/share"), | ||||||||||||||||||||
| Err(_err) => home_dir.join(".local/lib"), | ||||||||||||||||||||
| }; | ||||||||||||||||||||
| Ok(data_dir.join("tiles/server")) | ||||||||||||||||||||
| Ok(data_dir.join("tiles")) | ||||||||||||||||||||
| } | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
Comment on lines
+63
to
72
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# First, find and examine the file to verify the actual code structure
echo "=== Checking file structure and line numbers ==="
wc -l tiles/src/utils/config.rs
echo ""
echo "=== Lines 63-75 context ==="
sed -n '60,80p' tiles/src/utils/config.rs
echo ""
echo "=== Lines 100-115 (checking for is_memory_model) ==="
sed -n '100,115p' tiles/src/utils/config.rs
echo ""
echo "=== Searching for get_lib_dir and XDG_DATA_HOME usage ==="
rg -n "get_lib_dir|XDG_DATA_HOME" --type=rust -A2 -B2Repository: tilesprivacy/tiles Length of output: 3228 🌐 Web query:
💡 Result: Default XDG base directories (used if the corresponding environment variable is unset or empty):
Sources: XDG Base Directory Specification and reference implementations. [1][2] XDG_DATA_HOME fallback path is inconsistent with XDG spec and internal patterns. Using Additionally, the 🤖 Prompt for AI Agents |
||||||||||||||||||||
| pub fn get_config_dir() -> Result<PathBuf> { | ||||||||||||||||||||
|
|
@@ -100,3 +100,10 @@ pub fn get_data_dir() -> Result<PathBuf> { | |||||||||||||||||||
| Ok(data_dir.join("tiles")) | ||||||||||||||||||||
| } | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| pub fn is_memory_model(modelname: &str) -> bool { | ||||||||||||||||||||
| if modelname.contains("mem") { | ||||||||||||||||||||
| return true; | ||||||||||||||||||||
| } | ||||||||||||||||||||
| return false; | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
Comment on lines
+102
to
+107
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix clippy warning and simplify the function. The function can be simplified to a single expression, which also addresses the pipeline failure flagging the unneeded Proposed fix pub fn is_memory_model(modelname: &str) -> bool {
- if modelname.contains("mem") {
- return true;
- }
- return false;
+ modelname.contains("mem")
}📝 Committable suggestion
Suggested change
🧰 Tools🪛 GitHub Actions: Rust[error] 108-108: clippy: unneeded 🤖 Prompt for AI Agents |
||||||||||||||||||||
Uh oh!
There was an error while loading. Please reload this page.