Skip to content

Commit 960c46d

Browse files
authored
impr (cli + hql): fixing upserts, updates, and unique edges + improving CLI init command (#880)
<!-- greptile_comment --> <h3>Greptile Summary</h3> This PR fixes three core correctness issues — unique edge enforcement, update-from-variable codegen, and upsert default-value handling — and improves the CLI `init` command. **Key changes:** - **`add_e.rs`**: Rewrites unique-edge enforcement from the overly strict `PutFlags::NO_OVERWRITE` (which blocked any second same-label edge from a node regardless of target) to an explicit scan of existing `out_edges_db` entries. The new logic correctly enforces uniqueness only on the `(from, label, to)` triple. Tests confirm multiple targets and sources are allowed. - **`upsert.rs` / `traversal_steps.rs` / `traversal_validation.rs`**: Adds `upsert_*_with_defaults` variants that apply schema-defined `DEFAULT` values only during creation; updates correctly ignore them. The code generator now emits `upsert_*_with_defaults(...)` calls with defaults extracted from schema at analysis time. `TraversalType::Update` refactored to carry `source`/`source_is_plural`, enabling direct variable-to-update codegen without an intermediate read-only traversal. - **`init.rs`**: Changes `.gitignore` creation from unconditional overwrite to append with line deduplication. However, two bugs remain: (1) unconditional cleanup tracking even when file pre-existed, risking deletion of user content on failure; (2) no guard newline when appending to files lacking a trailing newline, potentially corrupting the last entry. - **Test additions**: New unit tests for unique-edge semantics, `upsert_*_with_defaults` behaviour, and two HQL integration test suites (`update_from_var`, `user_test_13`). **Findings:** - `.gitignore` append logic has two correctness issues that can result in data loss or file corruption. <details><summary><h3>Flowchart</h3></summary> ```mermaid %%{init: {'theme': 'neutral'}}%% flowchart TD A["add_edge called"] --> B{is_unique flag set?} B -->|Yes| C["Scan out_edges_db for from+label key"] C --> D{Duplicate to_node found?} D -->|Yes| E["Return DuplicateKey error"] D -->|No| F["Allocate Edge with v6 UUID"] B -->|No| F F --> G["Write to edges_db with APPEND"] G --> H["Write to out_edges_db with APPEND_DUP"] H --> I["Write to in_edges_db with APPEND_DUP"] I --> J["Return Ok with Edge value"] K["upsert_n_with_defaults called"] --> L{Iterator yields item?} L -->|Node exists| M["Update path: apply props only\nignore create_defaults"] L -->|Empty iterator| N["Create path: merge_create_props\nprops take priority over defaults"] L -->|Error| O["Propagate error"] N --> P["Insert new node with merged props"] M --> Q["Persist updated node"] R["UPDATE from variable"] --> S{Source variable type?} S -->|Single node var| T["G::new_mut_from with cloned var"] S -->|Collection var| U["G::new_mut_from_iter with cloned iter"] S -->|Inline traversal| V["Read-only traversal first then write"] T --> W[".update then collect_to_obj"] U --> W V --> W ``` </details> <sub>Last reviewed commit: 4e46111</sub> <!-- greptile_other_comments_section --> <!-- /greptile_comment -->
2 parents 75fd5dd + 7e7356a commit 960c46d

32 files changed

Lines changed: 3484 additions & 1100 deletions

File tree

Cargo.lock

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

helix-cli/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[package]
22
name = "helix-cli"
3-
version = "2.3.1"
3+
version = "2.3.2"
44
edition = "2024"
55

66
[dependencies]

helix-cli/src/commands/init.rs

Lines changed: 30 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ use eyre::Result;
1616
use std::collections::HashMap;
1717
use std::env;
1818
use std::fs;
19+
use std::io::Write;
1920
use std::path::Path;
2021

2122
pub async fn run(
@@ -338,14 +339,36 @@ fn create_project_structure(
338339
fs::write(&queries_path_file, default_queries)?;
339340
cleanup_tracker.track_file(queries_path_file);
340341

341-
// Create .gitignore
342-
let gitignore = r#".helix/
343-
target/
344-
*.log
345-
"#;
342+
// add this to .gitignore
343+
let gitignore = [".helix/", "target/", "*.log"];
346344
let gitignore_path = project_dir.join(".gitignore");
347-
fs::write(&gitignore_path, gitignore)?;
348-
cleanup_tracker.track_file(gitignore_path);
345+
let file_existed = gitignore_path.exists();
346+
let existing = fs::read_to_string(&gitignore_path).unwrap_or_default();
347+
348+
let missing_entries: Vec<&str> = gitignore
349+
.iter()
350+
.copied()
351+
.filter(|entry| !existing.lines().any(|line| line.trim() == *entry))
352+
.collect();
353+
354+
if !missing_entries.is_empty() {
355+
let mut file = fs::OpenOptions::new()
356+
.create(true)
357+
.append(true)
358+
.open(&gitignore_path)?;
359+
360+
if !existing.is_empty() && !existing.ends_with('\n') {
361+
writeln!(file)?;
362+
}
363+
364+
for entry in missing_entries {
365+
writeln!(file, "{entry}")?;
366+
}
367+
}
368+
369+
if !file_existed {
370+
cleanup_tracker.track_file(gitignore_path);
371+
}
349372

350373
Ok(())
351374
}

helix-cli/src/commands/integrations/helix.rs

Lines changed: 88 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,34 @@ pub struct HelixManager<'a> {
4242
project: &'a ProjectContext,
4343
}
4444

45+
fn build_standard_deploy_payload(
46+
schema_content: String,
47+
queries_map: HashMap<String, String>,
48+
cluster_name: &str,
49+
cluster_info: &CloudInstanceConfig,
50+
helix_toml_content: Option<String>,
51+
build_mode_override: Option<String>,
52+
) -> Result<serde_json::Value> {
53+
let build_mode = match cluster_info.build_mode {
54+
BuildMode::Dev => "dev",
55+
BuildMode::Release => "release",
56+
BuildMode::Debug => {
57+
return Err(eyre!("debug build mode is not supported for cloud deploys"));
58+
}
59+
};
60+
61+
Ok(json!({
62+
"schema": schema_content,
63+
"queries": queries_map,
64+
"env_vars": cluster_info.env_vars.clone(),
65+
"runtime_config": cluster_info.runtime_config(),
66+
"build_mode": build_mode,
67+
"instance_name": cluster_name,
68+
"helix_toml": helix_toml_content,
69+
"build_mode_override": build_mode_override,
70+
}))
71+
}
72+
4573
impl<'a> HelixManager<'a> {
4674
pub fn new(project: &'a ProjectContext) -> Self {
4775
Self { project }
@@ -58,7 +86,7 @@ impl<'a> HelixManager<'a> {
5886
let cluster_id = "YOUR_CLUSTER_ID".to_string();
5987

6088
// Use provided region or default to us-east-1
61-
let region = region.or_else(|| Some("us-east-1".to_string()));
89+
let region = region.or(Some("us-east-1".to_string()));
6290

6391
Ok(CloudInstanceConfig {
6492
cluster_id,
@@ -140,7 +168,7 @@ impl<'a> HelixManager<'a> {
140168
build_mode_override: Option<BuildMode>,
141169
) -> Result<()> {
142170
let credentials = require_auth().await?;
143-
let path = match get_path_or_cwd(path.as_ref()) {
171+
let path = match get_path_or_cwd(path.as_deref()) {
144172
Ok(path) => path,
145173
Err(e) => {
146174
return Err(eyre!("Error: failed to get path: {e}"));
@@ -241,22 +269,22 @@ impl<'a> HelixManager<'a> {
241269
// Prepare deployment payload
242270
let build_mode_override = build_mode_override
243271
.map(|mode| match mode {
244-
BuildMode::Dev => Ok("dev"),
245-
BuildMode::Release => Ok("release"),
272+
BuildMode::Dev => Ok("dev".to_string()),
273+
BuildMode::Release => Ok("release".to_string()),
246274
BuildMode::Debug => {
247275
Err(eyre!("debug build mode is not supported for cloud deploys"))
248276
}
249277
})
250278
.transpose()?;
251279

252-
let payload = json!({
253-
"schema": schema_content,
254-
"queries": queries_map,
255-
"env_vars": cluster_info.env_vars,
256-
"instance_name": cluster_name,
257-
"helix_toml": helix_toml_content,
258-
"build_mode_override": build_mode_override
259-
});
280+
let payload = build_standard_deploy_payload(
281+
schema_content,
282+
queries_map,
283+
&cluster_name,
284+
cluster_info,
285+
helix_toml_content,
286+
build_mode_override,
287+
)?;
260288

261289
// Initiate deployment with SSE streaming
262290
let client = reqwest::Client::new();
@@ -525,7 +553,7 @@ impl<'a> HelixManager<'a> {
525553
config: &crate::config::EnterpriseInstanceConfig,
526554
) -> Result<()> {
527555
let credentials = require_auth().await?;
528-
let path = match get_path_or_cwd(path.as_ref()) {
556+
let path = match get_path_or_cwd(path.as_deref()) {
529557
Ok(path) => path,
530558
Err(e) => {
531559
return Err(eyre!("Error: failed to get path: {e}"));
@@ -540,8 +568,7 @@ impl<'a> HelixManager<'a> {
540568
for entry in std::fs::read_dir(&queries_dir)? {
541569
let entry = entry?;
542570
let file_path = entry.path();
543-
if file_path.is_file() && file_path.extension().map(|e| e == "rs").unwrap_or(false)
544-
{
571+
if file_path.is_file() && file_path.extension().is_some_and(|e| e == "rs") {
545572
let filename = file_path.file_name().unwrap().to_string_lossy().to_string();
546573
let content = std::fs::read_to_string(&file_path)
547574
.map_err(|e| eyre!("Failed to read {}: {}", filename, e))?;
@@ -682,9 +709,54 @@ impl<'a> HelixManager<'a> {
682709
}
683710

684711
/// Returns the path or the current working directory if no path is provided
685-
pub fn get_path_or_cwd(path: Option<&String>) -> Result<PathBuf> {
712+
pub fn get_path_or_cwd(path: Option<&str>) -> Result<PathBuf> {
686713
match path {
687714
Some(p) => Ok(PathBuf::from(p)),
688715
None => Ok(env::current_dir()?),
689716
}
690717
}
718+
719+
#[cfg(test)]
720+
mod tests {
721+
use super::*;
722+
723+
#[test]
724+
fn build_standard_deploy_payload_includes_runtime_config_and_build_mode() {
725+
let mut queries = HashMap::new();
726+
queries.insert("search.hx".to_string(), "GetUsers {}".to_string());
727+
728+
let mut env_vars = HashMap::new();
729+
env_vars.insert("OPENAI_API_KEY".to_string(), "key".to_string());
730+
731+
let config = CloudInstanceConfig {
732+
cluster_id: "cluster-123".to_string(),
733+
region: Some("us-east-1".to_string()),
734+
build_mode: BuildMode::Release,
735+
env_vars,
736+
db_config: DbConfig {
737+
vector_config: crate::config::VectorConfig {
738+
db_max_size_gb: 42,
739+
..Default::default()
740+
},
741+
..Default::default()
742+
},
743+
};
744+
745+
let payload = build_standard_deploy_payload(
746+
"schema.hx".to_string(),
747+
queries,
748+
"prod",
749+
&config,
750+
Some("[project]\nname = \"demo\"\n".to_string()),
751+
Some("dev".to_string()),
752+
)
753+
.expect("payload should serialize");
754+
755+
assert_eq!(payload["build_mode"], "release");
756+
assert_eq!(payload["build_mode_override"], "dev");
757+
assert_eq!(payload["instance_name"], "prod");
758+
assert_eq!(payload["env_vars"]["OPENAI_API_KEY"], "key");
759+
assert_eq!(payload["runtime_config"]["db_max_size_gb"], 42);
760+
assert!(payload["helix_toml"].is_string());
761+
}
762+
}

0 commit comments

Comments
 (0)