Skip to content

Commit ca40ed2

Browse files
authored
impr(cli + core + hql): ordered return values, helix restart, hql fixes, api verification (#841)
<!-- greptile_comment --> <h2>Greptile Overview</h2> <h3>Greptile Summary</h3> This PR implements several major improvements across the CLI, core database, and HQL compiler components. **Key Changes:** - **CLI Output System**: introduced a new structured output module (`output.rs`) with verbosity control (quiet/normal/verbose), modern spinner UI, and operation tracking - significantly improves developer experience - **Restart Command**: added `helix restart` command supporting both local Docker instances and cloud deployments (Fly.io) - **Ordered Return Values**: implemented ordered return values in HQL queries using a struct-based approach instead of json! macros, preserving field order as defined in queries - **Variable Binding Fix**: fixed bug where variable bindings and map variables weren't appearing in inline RETURN objects - now properly handles scope variables vs schema properties - **Test Infrastructure**: added comprehensive test utilities with isolated temp directories and extensive lifecycle tests for build/start/stop/restart workflows - **API Key Verification**: added conditional API key verification for schema introspection endpoint with feature flag support **Technical Improvements:** - Switched from `HashMap` to `IndexMap` in object validation to preserve field ordering - Enhanced return value generation to distinguish primitive types (Count/Boolean/Scalar) from complex types (Nodes/Edges/Objects) - Improved nested traversal handling with variable reference detection - Updated all CLI commands to use new `Output`/`Step` helpers for consistent UX **Critical Issues:** The test utilities use `unsafe` blocks to modify environment variables, claiming safety based on isolation. However, environment variables are process-global and this creates actual data races when tests run in parallel. The `unsafe` blocks should be removed - the operations aren't actually unsafe in Rust's memory safety sense, but the concurrency issue remains. <details><summary><h3>Important Files Changed</h3></summary> | Filename | Overview | |----------|----------| | helix-cli/src/tests/test_utils.rs | new test utilities with isolated temp directories and env variable management | | helix-db/src/helixc/generator/queries.rs | implemented ordered return values with struct-based approach and variable reference handling | | helix-db/src/helixc/analyzer/methods/query_validation.rs | fixed variable binding and map variable handling in return objects, added primitive type detection | | helix-db/src/helixc/analyzer/methods/object_validation.rs | improved scope variable handling in property access, switched HashMap to IndexMap for ordering | </details> </details> <details><summary><h3>Sequence Diagram</h3></summary> ```mermaid sequenceDiagram participant User participant CLI participant Output participant Docker participant Compiler participant Generator participant Gateway User->>CLI: helix restart dev CLI->>Output: Operation::new("Restarting", "dev") Output-->>User: Display operation header CLI->>Docker: restart_instance() Docker->>Docker: check_runtime_available() Docker->>Docker: verify docker-compose.yml exists Docker->>Output: Step::start("Restarting container") Docker->>Docker: run_compose_command(["restart"]) Docker->>Output: Step::done() Output-->>User: Display success Note over User,Gateway: Ordered Return Values Flow User->>CLI: helix build dev CLI->>Compiler: compile HQL queries Compiler->>Compiler: analyze_return_expr() Compiler->>Compiler: build_return_fields() Compiler->>Compiler: detect primitive vs struct types alt Primitive Type (Count/Boolean/Scalar) Compiler->>Generator: create primitive ReturnValueStruct Generator->>Generator: emit variable directly (no struct def) else Complex Type (Nodes/Edges/Objects) Compiler->>Generator: create nested ReturnValueStruct Generator->>Generator: generate struct definitions Generator->>Generator: handle variable references Generator->>Generator: map closure parameters end Generator->>Generator: format ordered return JSON Generator-->>CLI: compiled Rust code CLI->>Docker: build_instance() Docker-->>User: Built instance Note over User,Gateway: API Key Verification User->>Gateway: introspect_schema request Gateway->>Gateway: check API key feature flag alt API Key Enabled Gateway->>Gateway: validate API key alt Valid Key Gateway-->>User: return schema else Invalid Key Gateway-->>User: 401 Unauthorized end else API Key Disabled Gateway-->>User: return schema end ``` </details> <!-- greptile_other_comments_section --> <!-- /greptile_comment -->
2 parents 47c7e22 + abf1d1d commit ca40ed2

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

83 files changed

+3909
-1338
lines changed

.github/workflows/cli_tests.yml

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
# name: CLI Tests
2+
3+
# on:
4+
# pull_request:
5+
# branches: [main, dev]
6+
# paths:
7+
# - 'helix-cli/**'
8+
# - 'helix-macros/**'
9+
# - 'helix-container/**'
10+
# - 'helix-db/**'
11+
# - 'Cargo.toml'
12+
# - 'Cargo.lock'
13+
14+
# env:
15+
# CARGO_TERM_COLOR: always
16+
# RUST_BACKTRACE: 1
17+
18+
# jobs:
19+
# test:
20+
# name: Test (${{ matrix.os }})
21+
# runs-on: ${{ matrix.os }}
22+
# strategy:
23+
# fail-fast: false
24+
# matrix:
25+
# os: [ubuntu-latest, macos-latest, windows-latest]
26+
27+
# steps:
28+
# - name: Checkout repository
29+
# uses: actions/checkout@v4
30+
31+
# - name: Setup Rust toolchain
32+
# uses: dtolnay/rust-toolchain@stable
33+
34+
# - name: Cache cargo registry
35+
# uses: actions/cache@v4
36+
# with:
37+
# path: |
38+
# ~/.cargo/registry
39+
# ~/.cargo/git
40+
# target
41+
# key: ${{ runner.os }}-cargo-${{ hashFiles('**/Cargo.lock') }}
42+
# restore-keys: |
43+
# ${{ runner.os }}-cargo-
44+
45+
# - name: Run helix-cli unit tests
46+
# run: cargo test --release --package helix-cli
47+
# env:
48+
# # Use a unique temp directory for test isolation
49+
# HELIX_CACHE_DIR: ${{ runner.temp }}/helix-test-cache-${{ github.run_id }}
50+
51+
# - name: Run helix-macros tests
52+
# run: cargo test --release --package helix-macros
53+
54+
# # Integration tests that require repo cloning run serially
55+
# - name: Run helix-cli integration tests (ignored tests)
56+
# run: cargo test --release --package helix-cli -- --ignored --test-threads=1
57+
# env:
58+
# HELIX_CACHE_DIR: ${{ runner.temp }}/helix-test-cache-ignored-${{ github.run_id }}

.github/workflows/db_tests.yml

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,11 @@ name: Core Database Tests
33
on:
44
pull_request:
55
branches: [ main, dev ]
6+
paths:
7+
- 'helix-macros/**'
8+
- 'helix-db/**'
9+
- 'Cargo.toml'
10+
- 'Cargo.lock'
611

712
jobs:
813
test:
@@ -33,6 +38,6 @@ jobs:
3338
${{ runner.os }}-cargo-
3439

3540
- name: Run tests
36-
run: |
41+
run: |
3742
cd helix-db
3843
cargo test --release --lib -- --skip concurrency_tests

.github/workflows/dev_instance_tests.yml

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,12 @@ name: Core Database Tests In Dev Instance Mode
33
on:
44
pull_request:
55
branches: [ main, dev ]
6+
paths:
7+
- 'helix-macros/**'
8+
- 'helix-db/**'
9+
- 'Cargo.toml'
10+
- 'Cargo.lock'
11+
612

713
jobs:
814
test:
@@ -33,6 +39,6 @@ jobs:
3339
${{ runner.os }}-cargo-
3440

3541
- name: Run dev instance tests
36-
run: |
42+
run: |
3743
cd helix-db
3844
cargo test --release --lib --features dev-instance -- --skip concurrency_tests

.github/workflows/hql_tests.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,9 @@ name: HQL Tests
33
on:
44
pull_request:
55
branches: [main, dev]
6+
paths:
7+
- 'helix-macros/**'
8+
- 'helix-db/**'
69

710
jobs:
811
hql-tests:

.github/workflows/production_db_tests.yml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,11 @@ name: Core Database Tests In Production Mode
33
on:
44
pull_request:
55
branches: [ main, dev ]
6+
paths:
7+
- 'helix-macros/**'
8+
- 'helix-db/**'
9+
- 'Cargo.toml'
10+
- 'Cargo.lock'
611

712
jobs:
813
test:

Cargo.lock

Lines changed: 35 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: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[package]
22
name = "helix-cli"
3-
version = "2.2.5"
3+
version = "2.2.6"
44
edition = "2024"
55

66
[dependencies]
@@ -36,6 +36,7 @@ crossterm = "0.28"
3636

3737
[dev-dependencies]
3838
tempfile = "3.23.0"
39+
serial_test = "3.2"
3940

4041
[lib]
4142
name = "helix_cli"

helix-cli/build.sh

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,9 @@ export PATH="$HOME/.cargo/bin:$PATH"
77

88
# if dev profile, build with dev profile
99
if [ "$1" = "dev" ]; then
10-
cargo install --profile dev --force --path . --root ~/.local
10+
cargo install --debug --force --path . --root ~/.local
1111
else
12-
cargo install --release --force --path . --root ~/.local
12+
cargo install --force --path . --root ~/.local
1313
fi
1414

1515
if ! echo "$PATH" | grep -q "$HOME/.local/bin"; then

helix-cli/src/commands/add.rs

Lines changed: 12 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,10 @@ use crate::commands::integrations::helix::HelixManager;
66
use crate::config::{BuildMode, CloudConfig, DbConfig, LocalInstanceConfig};
77
use crate::docker::DockerManager;
88
use crate::errors::project_error;
9+
use crate::output::{Operation, Step};
910
use crate::project::ProjectContext;
1011
use crate::prompts;
11-
use crate::utils::{print_instructions, print_status, print_success};
12+
use crate::utils::print_instructions;
1213
use eyre::Result;
1314
use std::env;
1415

@@ -84,10 +85,7 @@ async fn run_add_inner(
8485
.into());
8586
}
8687

87-
print_status(
88-
"ADD",
89-
&format!("Adding instance '{instance_name}' to Helix project"),
90-
);
88+
let op = Operation::new("Adding", &instance_name);
9189

9290
// Backup the original config before any modifications
9391
let config_path = project_context.root.join("helix.toml");
@@ -115,7 +113,7 @@ async fn run_add_inner(
115113
let config_path = project_context.root.join("helix.toml");
116114
project_context.config.save_to_file(&config_path)?;
117115

118-
print_status("CLOUD", "Helix cloud instance configuration added");
116+
Step::verbose_substep("Helix cloud instance configuration added");
119117

120118
// Prompt user to create cluster now
121119
println!();
@@ -140,9 +138,7 @@ async fn run_add_inner(
140138

141139
// create_cluster::run() already saved the updated config with the real cluster_id
142140
// Return early to avoid overwriting it with the stale in-memory config
143-
print_success(&format!(
144-
"Instance '{instance_name}' added to Helix project"
145-
));
141+
op.success();
146142

147143
print_instructions(
148144
"Next steps:",
@@ -159,13 +155,10 @@ async fn run_add_inner(
159155
return Ok(());
160156
} else {
161157
println!();
162-
print_status(
163-
"INFO",
164-
&format!(
165-
"Cluster creation skipped. Run 'helix create-cluster {}' when ready.",
166-
instance_name
167-
),
168-
);
158+
crate::output::info(&format!(
159+
"Cluster creation skipped. Run 'helix create-cluster {}' when ready.",
160+
instance_name
161+
));
169162
}
170163
}
171164
CloudDeploymentTypeCommand::Ecr { .. } => {
@@ -196,7 +189,7 @@ async fn run_add_inner(
196189
.cloud
197190
.insert(instance_name.clone(), CloudConfig::Ecr(ecr_config.clone()));
198191

199-
print_status("ECR", "AWS ECR repository initialized successfully");
192+
Step::verbose_substep("AWS ECR repository initialized successfully");
200193
}
201194
CloudDeploymentTypeCommand::Fly {
202195
auth,
@@ -246,17 +239,15 @@ async fn run_add_inner(
246239
.config
247240
.local
248241
.insert(instance_name.clone(), local_config);
249-
print_status("LOCAL", "Local instance configuration added");
242+
Step::verbose_substep("Local instance configuration added");
250243
}
251244
}
252245

253246
// Save the updated configuration
254247
let config_path = project_context.root.join("helix.toml");
255248
project_context.config.save_to_file(&config_path)?;
256249

257-
print_success(&format!(
258-
"Instance '{instance_name}' added to Helix project"
259-
));
250+
op.success();
260251

261252
print_instructions(
262253
"Next steps:",

0 commit comments

Comments
 (0)