impr (hql+core+cli+mcp): cleaning up cli, adding mcp aggregation methods, v from id/type #438

xav-db · 2025-09-16T03:36:46Z

Description

Related Issues

Checklist when merging to main

No compiler warnings (if applicable)
Code is formatted with rustfmt
No useless or dead code (if applicable)
Code is easy to understand
Doc comments are used for all functions, enums, structs, and fields (where appropriate)
All tests pass
Performance has not regressed (assuming change was not to fix a bug)
Version number has been updated in helix-cli/Cargo.toml and helixdb/Cargo.toml

Additional Notes

…ncatenated content

## Description - Remove Label field names since label is in RESERVED_FIELD_NAMES - Change the Integer type to the appropriate int type definition - Change user_id to ID ## Related Issues ### None Closes # ## Checklist when merging to main - [x] No compiler warnings (if applicable) - [x] Code is formatted with `rustfmt` - [x] No useless or dead code (if applicable) - [x] Code is easy to understand - [x] Doc comments are used for all functions, enums, structs, and fields (where appropriate) - [x] All tests pass - [x] Performance has not regressed (assuming change was not to fix a bug) - [ ] Version number has been updated in `helix-cli/Cargo.toml` and `helixdb/Cargo.toml` ## Additional Notes ### None

…to schema parsing methods (#429) ## Description ### Problem Diagnostics displayed incorrect line content because the diagnostic renderer used combined source content instead of the specific file content where the error occurred. Before (incorrect): ```sh error[E204]: field `Label` is a reserved field name ┌─ /home/pro/test/helix/second/queries.hx:3:6 │ 3 │ RETURN friends ← Wrong content from query.hx file instead of the schema.hx │ ^^^^^^^^^^^^^ ``` ### Solution - Added filepath parameter to schema parsing methods (parse_field_def, parse_node_body, parse_properties) - Updated diagnostic rendering in helix-cli/src/utils.rs to find the correct file content by matching the diagnostic's filepath: ```rs // Find the correct file content for this diagnostic let file_content = content.files.iter() .find(|f| f.name == filepath) .map(|f| &f.content) .unwrap_or(&analyzed_source.src); ``` After (correct): ```rs error[E204]: field `Label` is a reserved field name ┌─ /home/pro/test/helix/second/schema.hx:3:6 │ 3 │ Label: String, ← Correct content from schema file │ ^^^^^^^^^^^^^ ``` ### summary • Added filepath parameter to schema parsing methods (parse_field_def, parse_node_body, parse_properties) • Updated location creation to use loc_with_filepath(filepath) instead of loc() • Fixed diagnostic rendering to use individual file content matching the diagnostic's filepath • Updated CLI integration to pass Content structure for proper file access ## Related Issues ### None ## Checklist when merging to main - [x] No compiler warnings (if applicable) - [x] Code is formatted with `rustfmt` - [x] No useless or dead code (if applicable) - [x] Code is easy to understand - [x] Doc comments are used for all functions, enums, structs, and fields (where appropriate) - [x] All tests pass - [x] Performance has not regressed (assuming change was not to fix a bug) - [ ] Version number has been updated in `helix-cli/Cargo.toml` and `helixdb/Cargo.toml` ## Additional Notes ### None

…g for return

## Description This PR adds validation for field types in schema definitions to prevent the use of unknown or custom types (identifiers) in node, vector, and edge fields. It enforces the use of built-in types only. Previously, invalid type definitions could still pass the Helix check ## Related Issues ### None ## Checklist when merging to main - [x] No compiler warnings (if applicable) - [x] Code is formatted with `rustfmt` - [x] No useless or dead code (if applicable) - [x] Code is easy to understand - [x] Doc comments are used for all functions, enums, structs, and fields (where appropriate) - [x] All tests pass - [x] Performance has not regressed (assuming change was not to fix a bug) - [ ] Version number has been updated in `helix-cli/Cargo.toml` and `helixdb/Cargo.toml` ## Additional Notes ### None

## Description  ## Related Issues  Closes # ## Checklist when merging to main  - [ ] No compiler warnings (if applicable) - [ ] Code is formatted with `rustfmt` - [ ] No useless or dead code (if applicable) - [ ] Code is easy to understand - [ ] Doc comments are used for all functions, enums, structs, and fields (where appropriate) - [ ] All tests pass - [ ] Performance has not regressed (assuming change was not to fix a bug) - [ ] Version number has been updated in `helix-cli/Cargo.toml` and `helixdb/Cargo.toml` ## Additional Notes

## Description  ## Related Issues  Closes #409 ## Checklist when merging to main  - [ ] No compiler warnings (if applicable) - [ ] Code is formatted with `rustfmt` - [ ] No useless or dead code (if applicable) - [ ] Code is easy to understand - [ ] Doc comments are used for all functions, enums, structs, and fields (where appropriate) - [ ] All tests pass - [ ] Performance has not regressed (assuming change was not to fix a bug) - [ ] Version number has been updated in `helix-cli/Cargo.toml` and `helixdb/Cargo.toml` ## Additional Notes

Copilot

Pull Request Overview

This PR implements cleaning up CLI functionality, adding MCP aggregation methods, and enabling vector retrieval from ID/type. The changes enhance the database's aggregation capabilities and vector handling while improving CLI usability.

Adds new MCP aggregation and grouping functionality for data analysis
Implements vector lookup by ID and type similar to existing node/edge operations
Refactors CLI error handling and content processing to improve user experience

Reviewed Changes

Copilot reviewed 34 out of 36 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
hql-tests/tests/negating_exists/schema.hx	Updates example schema to use modern types (I32, Date)
hql-tests/tests/negating_exists/queries.hx	Updates example query parameter type to ID
helix-db/src/utils/mod.rs	Adds new utility modules for aggregation and grouping
helix-db/src/utils/group_by.rs	Implements GroupBy data structure for organizing results
helix-db/src/utils/count.rs	Adds ShouldCount enum for counting operations
helix-db/src/utils/aggregate.rs	Implements Aggregate data structure for aggregating results
helix-db/src/protocol/return_values.rs	Refactors return value processing and adds new conversion methods
helix-db/src/lib.rs	Adds mimalloc as global allocator
helix-db/src/helixc/parser/types.rs	Adds Vector start node type and Aggregate/GroupBy step types
helix-db/src/helixc/parser/schema_parse_methods.rs	Updates schema parsing to include filepath information
helix-db/src/helixc/parser/graph_step_parse_methods.rs	Adds parsing for aggregate and group_by operations
helix-db/src/helixc/generator/traversal_steps.rs	Implements code generation for GroupBy and AggregateBy steps
helix-db/src/helixc/generator/source_steps.rs	Adds VFromID and VFromType source step generation
helix-db/src/helixc/generator/return_values.rs	Updates return value generation method names
helix-db/src/helixc/analyzer/types.rs	Improves Display formatting
helix-db/src/helixc/analyzer/methods/traversal_validation.rs	Adds validation for vector operations and aggregation steps
helix-db/src/helixc/analyzer/methods/schema_methods.rs	Adds validation for schema field types
helix-db/src/helix_gateway/mcp/tools.rs	Adds order_by functionality and refactors search methods
helix-db/src/helix_gateway/mcp/mcp.rs	Implements MCP handlers for aggregate_by and group_by operations
helix-db/src/helix_engine/vector_core/vector_core.rs	Adds documentation for vector key format
helix-db/src/helix_engine/vector_core/vector.rs	Adds version field and label retrieval methods to HVector
helix-db/src/helix_engine/traversal_core/traversal_value.rs	Adds get_properties method to Traversable trait
helix-db/src/helix_engine/traversal_core/traversal_iter.rs	Removes unused count_to_val methods
helix-db/src/helix_engine/traversal_core/ops/util/mod.rs	Adds new utility operation modules
helix-db/src/helix_engine/traversal_core/ops/util/group_by.rs	Implements GroupByAdapter trait for grouping operations
helix-db/src/helix_engine/traversal_core/ops/util/count.rs	Implements CountAdapter trait for counting operations
helix-db/src/helix_engine/traversal_core/ops/util/aggregate.rs	Implements AggregateAdapter trait for aggregation operations
helix-db/src/helix_engine/traversal_core/ops/source/v_from_type.rs	Implements vector lookup by type functionality
helix-db/src/helix_engine/traversal_core/ops/source/mod.rs	Adds v_from_type module
helix-db/src/helix_engine/tests/traversal_tests/count_tests.rs	Updates imports for CountAdapter
helix-db/src/helix_engine/bm25/bm25.rs	Adds temporary BM25 configuration creation
helix-db/src/grammar.pest	Adds grammar rules for AGGREGATE_BY and GROUP_BY
helix-db/Cargo.toml	Adds rayon and mimalloc dependencies
helix-cli/src/utils.rs	Improves error handling and file content processing

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-09-17T19:09:05Z

helix-db/src/helix_engine/vector_core/vector.rs

+        match &self.properties {
+            Some(p) => p.get("label"),
+            None => None
+        } 


This method can be simplified using Option::and_then: self.properties.as_ref().and_then(|p| p.get(\"label\"))

Suggested change

match &self.properties {

Some(p) => p.get("label"),

None => None

}

self.properties.as_ref().and_then(|p| p.get("label"))

Copilot · 2025-09-17T19:09:06Z

helix-db/src/helix_gateway/mcp/tools.rs

+            );
+            println!(
+                "Error checking BM25 metadata: {e:?} - returning empty results",
+


Empty line inside println! macro call creates unnecessary whitespace. Remove the empty line between the format string and the closing parenthesis.

Suggested change

Copilot · 2025-09-17T19:09:06Z

helix-db/src/helix_engine/traversal_core/ops/util/group_by.rs

+        for item in self.inner {
+            let item = item?;
+
+            // TODO HANDLE COUNT


TODO comment should provide more specific details about what count handling needs to be implemented and when it should be addressed.

Suggested change

// TODO HANDLE COUNT

Copilot · 2025-09-17T19:09:07Z

helix-db/src/helix_engine/traversal_core/ops/util/aggregate.rs

+        for item in self.inner {
+            let item = item?;
+
+            // TODO HANDLE COUNT


TODO comment should provide more specific details about what count handling needs to be implemented and when it should be addressed.

Suggested change

// TODO HANDLE COUNT

Copilot · 2025-09-17T19:09:07Z

helix-db/src/helixc/generator/traversal_steps.rs

+}
+impl Display for GroupBy {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "group_by(&[{}])", self.properties.iter().map(|s|s.to_string()).collect::<Vec<_>>().join(","))


Unnecessary intermediate Vec allocation. Use self.properties.iter().map(|s| s.to_string()).collect::<Vec<_>>().join(\",\") directly or consider using itertools::join for better performance.

Copilot · 2025-09-17T19:09:07Z

helix-db/src/helixc/generator/traversal_steps.rs

+}
+impl Display for AggregateBy {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "aggregate_by(&[{}])", self.properties.iter().map(|s|s.to_string()).collect::<Vec<_>>().join(","))


Unnecessary intermediate Vec allocation. Use self.properties.iter().map(|s| s.to_string()).collect::<Vec<_>>().join(\",\") directly or consider using itertools::join for better performance.

…ixng #515

## Description  ## Related Issues  Closes #511 #512 #513 #514 #515 #516 #517 ## Checklist when merging to main  - [ ] No compiler warnings (if applicable) - [ ] Code is formatted with `rustfmt` - [ ] No useless or dead code (if applicable) - [ ] Code is easy to understand - [ ] Doc comments are used for all functions, enums, structs, and fields (where appropriate) - [ ] All tests pass - [ ] Performance has not regressed (assuming change was not to fix a bug) - [ ] Version number has been updated in `helix-cli/Cargo.toml` and `helixdb/Cargo.toml` ## Additional Notes

ishaksebsib and others added 29 commits September 7, 2025 21:43

fix: remove Label since it's in RESERVED_FIELD_NAMES

d988e9d

fix: change Integer type to appropriate type definitions

bd361d0

fix: change user_id type to ID

51fb39c

fix: pass original file content for diagnostic instead of combined/co…

bdace87

…ncatenated content

feat: Add filepath support to schema parsing methods

80964bb

feat: add schema field type validation

546273b

feat: add default now on since field

02a32cb

pushing order by as tool

e8280dd

pushing group by, and initial aggregation

8761707

pushing aggregate and group by and count.

fc62c39

fixing aggregate, need to implement into return value

16e4ef0

making group by use enum

b13f9f6

pushing conversion into return values. needs improvement

c83e91c

adding from aggregation

a846867

implementing single iteration rather than collecting before processin…

3628e94

…g for return

finalising aggregate and group by

138afe6

adding aggregate and group by to tools

9652108

v from id and v from type

7bfae99

fixing clippy

f254e9f

Merge branch 'dev' into implementing-v-from-id-and-type

fe80b43

fixing clippy

c657a11

implementing local keyword search

d923165

fixing clippy

719c01e

xav-db marked this pull request as ready for review September 17, 2025 19:07

xav-db requested review from RaphaelDarley, Copilot and matthewsanetra September 17, 2025 19:07

Copilot AI reviewed Sep 17, 2025

View reviewed changes

xav-db added 8 commits September 17, 2025 15:06

fixing issues with return values and date time comparison

38317ab

leaving array literal as field for now

b06349a

fixing aggregate and group by

c64762b

from n fixes from N issue [Bug]: FromN not working #514

c35a275

fixing issue with nested ands/ors not being correctly parenthesised f…

b1ef558

…ixng #515

fixing [Bug]: Property selection with renaming #516

ffe2669

fixing [Bug]: Property aliasing with spread operator #517

07fc913

removing queries

12c282b

matthewsanetra approved these changes Sep 18, 2025

View reviewed changes

xav-db and others added 3 commits September 18, 2025 12:27

fixing clippy

3c3f31f

updating cli version

afec30e

xav-db merged commit 175e1f1 into main Sep 20, 2025
13 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

impr (hql+core+cli+mcp): cleaning up cli, adding mcp aggregation methods, v from id/type #438

impr (hql+core+cli+mcp): cleaning up cli, adding mcp aggregation methods, v from id/type #438

Uh oh!

xav-db commented Sep 16, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Sep 17, 2025

Uh oh!

Copilot AI Sep 17, 2025

Uh oh!

Copilot AI Sep 17, 2025

Uh oh!

Copilot AI Sep 17, 2025

Uh oh!

Copilot AI Sep 17, 2025

Uh oh!

Copilot AI Sep 17, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

impr (hql+core+cli+mcp): cleaning up cli, adding mcp aggregation methods, v from id/type #438

impr (hql+core+cli+mcp): cleaning up cli, adding mcp aggregation methods, v from id/type #438

Uh oh!

Conversation

xav-db commented Sep 16, 2025

Description

Related Issues

Checklist when merging to main

Additional Notes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Sep 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 17, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants