Skip to content

feat: add HGraph no-footer serialization overload#1968

Open
wxyucs wants to merge 3 commits into
mainfrom
feat/hgraph-no-footer-serialization
Open

feat: add HGraph no-footer serialization overload#1968
wxyucs wants to merge 3 commits into
mainfrom
feat/hgraph-no-footer-serialization

Conversation

@wxyucs
Copy link
Copy Markdown
Collaborator

@wxyucs wxyucs commented May 6, 2026

Summary

  • Add explicit HGraph stream serialization/deserialization overloads that can skip footer handling.
  • Reuse the existing old serial format for no-footer HGraph payloads to support pure streaming scenarios.
  • Add unit coverage verifying no footer is emitted and deserialized search results match.

Tests

  • cmake --build ./build --target unittests --parallel 6
  • ./build/tests/unittests -d yes "HGraph serialize and deserialize without footer"
  • ./build/tests/unittests -d yes "[ut][HGraphParameter]"

Signed-off-by: Xiangyu Wang <wxy407827@antgroup.com>
Copilot AI review requested due to automatic review settings May 6, 2026 07:00
@mergify
Copy link
Copy Markdown
Contributor

mergify Bot commented May 6, 2026

Merge Protections

Your pull request matches the following merge protections and will not be merged until they are valid.

🔴 Require linked issue for feature/bug PRs

Waiting for

  • body~=(?im)(?:^|[\s\-\*])(?:close[sd]?|fix(?:e[sd])?|resolve[sd]?)\s*:?\s+(?:#\d+|[\w.\-]+/[\w.\-]+#\d+|https?://github\.com/[\w.\-]+/[\w.\-]+/issues/\d+)
This rule is failing.
  • body~=(?im)(?:^|[\s\-\*])(?:close[sd]?|fix(?:e[sd])?|resolve[sd]?)\s*:?\s+(?:#\d+|[\w.\-]+/[\w.\-]+#\d+|https?://github\.com/[\w.\-]+/[\w.\-]+/issues/\d+)

🟢 Require kind label

Wonderful, this rule succeeded.
  • label~=^kind/

🟢 Require version label

Wonderful, this rule succeeded.
  • label~=^version/

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the HGraph serialization and deserialization logic to support an optional footer. It introduces overloaded Serialize and Deserialize methods that accept a boolean flag to toggle the footer, and extracts core logic into helper methods like serialize_without_footer and deserialize_data to improve code reuse. Additionally, a new unit test was added to verify the correctness of the footer-less serialization path. I have no feedback to provide.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds explicit HGraph stream serialization/deserialization overloads that optionally skip footer handling, enabling “pure streaming” payloads by reusing the legacy (v0.14) no-footer layout.

Changes:

  • Added Serialize(writer, bool with_footer) / Deserialize(reader, bool with_footer) overloads and refactored legacy no-footer logic into helper functions.
  • Centralized shared deserialization steps into deserialize_data() to reduce duplication across formats.
  • Added a unit test to verify no footer is emitted and that search results match after no-footer roundtrip.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
src/algorithm/hgraph.h Declares new serialize/deserialize overloads and helper methods for no-footer handling.
src/algorithm/hgraph.cpp Implements footer/no-footer branching, extracts legacy no-footer (v0.14) helpers, and reuses common deserialization logic.
src/algorithm/hgraph_parameter_test.cpp Adds unit coverage for no-footer serialization/deserialization roundtrip correctness.

Comment thread src/algorithm/hgraph.cpp
Comment thread src/algorithm/hgraph.cpp
Comment thread src/algorithm/hgraph_parameter_test.cpp
wxyucs added 2 commits May 6, 2026 15:15
Signed-off-by: Xiangyu Wang <wxy407827@antgroup.com>
Signed-off-by: Xiangyu Wang <wxy407827@antgroup.com>
Copilot AI review requested due to automatic review settings May 6, 2026 09:30
@wxyucs
Copy link
Copy Markdown
Collaborator Author

wxyucs commented May 6, 2026

CircleCI failure was in the random DiskANN functional test: the randomized search returned fewer than requested top-k results under approximate/limited search parameters. Updated the test to assert DiskANN returns a non-empty result set bounded by the requested top-k instead of requiring exactly full top-k. Local verification passed: ./build/tests/functests -d yes "Test Random Index", ./build/tests/unittests -d yes "HGraph serialize and deserialize without footer", and ./build/tests/unittests -d yes "[ut][HGraphParameter]".

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

@wxyucs wxyucs self-assigned this May 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants