Skip to content

spike(storage): Arrow fixed-partition sort experiment (#1506) - #1511

Merged
DecisionNerd merged 1 commit into
mainfrom
spike/1506-sort-partition-reuse
Sep 20, 2026
Merged

DecisionNerd merged 1 commit into
mainfrom
spike/1506-sort-partition-reuse

Conversation

@DecisionNerd

@DecisionNerd DecisionNerd commented Sep 20, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Adds a test-support GF_SHAPE_SORT_SPIKE=arrow path that sorts fixed-width construction partition records with Arrow sort_to_indices + take.
  • Retains compact detail offset sorting (hybrid) and fails closed on invalid modes.
  • Records the pre-measurement protocol citation and partitioning-authority note in docs/development/evidence/shape-sort-partition-spike-1506.md.

Progresses #1506 (does not close — S18/S20 paired measurements and remaining SortExec/repartition candidates remain).

Test plan

Made with Cursor


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

Summary by CodeRabbit

  • Tests
    • Added test coverage for alternative sorting behavior on fixed-width records.
    • Verified that the experimental sorting path produces the same ordering as the standard path.
    • Added validation for unsupported configuration values and ensured existing behavior remains unchanged for other record types.
    • These changes are limited to test and test-support builds and do not alter production behavior.

Introduce a test-support GF_SHAPE_SORT_SPIKE=arrow path that sorts fixed
partition records via Arrow kernels while retaining compact detail sorting,
with equivalence tests and a pre-measurement evidence note.

Co-authored-by: Cursor <cursoragent@cursor.com>
@coderabbitai

coderabbitai Bot commented Sep 20, 2026 •

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: CurateLabs/graphforge/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: cf048940-673c-4f1c-ac6e-f923d46a2b39

📥 Commits

Reviewing files that changed from the base of the PR and between 7febcb4 and 6afba9a.

⛔ Files ignored due to path filters (1)
  • docs/development/evidence/shape-sort-partition-spike-1506.md is excluded by !**/*.md, !**/docs/**
📒 Files selected for processing (3)
  • crates/graphforge-storage/src/graph_construction/partition_records.rs
  • crates/graphforge-storage/src/graph_construction/partition_records/sort_spike.rs
  • crates/graphforge-storage/src/graph_construction/partition_shaping.rs

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


Walkthrough

The change adds a test-support sorting experiment for fixed partition records. The GF_SHAPE_SORT_SPIKE variable selects baseline or Arrow sorting. Detail records retain baseline sorting. Fixed partition loading uses the selector in test-support builds.

Changes

Partition sorting experiment

Layer / File(s) Summary
Sorting selector API
crates/graphforge-storage/src/graph_construction/partition_records.rs
Adds the cfg-gated sort_selected method and the sort_spike module. The detail helper is visible to the submodule.
Baseline and Arrow sorting
crates/graphforge-storage/src/graph_construction/partition_records/sort_spike.rs
Parses GF_SHAPE_SORT_SPIKE, applies baseline or Arrow sorting, rejects invalid modes, validates sorted output, and tests fixed and detail paths.
Partition loading integration
crates/graphforge-storage/src/graph_construction/partition_shaping.rs
Uses sort_selected in test-support builds and keeps sort for other builds.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant PartitionShaping
  participant PartitionRecords
  participant SortSpike
  participant Arrow
  PartitionShaping->>PartitionRecords: sort_selected()
  PartitionRecords->>SortSpike: apply(records)
  SortSpike->>Arrow: sort fixed records when mode is arrow
  Arrow-->>SortSpike: sorted records
  SortSpike-->>PartitionRecords: sorting result
Loading
🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description summarizes the Arrow sorting experiment and lists one completed test, but it does not follow the repository template. It omits the required type-of-change, changes-made, checklist, per… Complete the required template sections. Mark the applicable change types and checklist items, describe the changes and performance impact, document breaking-change status, record all testing and evidence review results, and complete the co…
Docstring Coverage ⚠️ Warning Docstring coverage is 38.46% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 13 functions across 3 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the storage sorting experiment and matches the main changes in the pull request.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description summarizes the Arrow sorting experiment and lists one completed test, but it does not follow the repository template. It omits the required type-of-change, changes-made, checklist, performance, breaking-changes, compliance, and contributor-confirmation sections, and it leaves planned validation incomplete.

Resolution

Complete the required template sections. Mark the applicable change types and checklist items, describe the changes and performance impact, document breaking-change status, record all testing and evidence review results, and complete the compliance and contributor confirmations.

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

Warning

Some tools did not complete. Review the errors below.

🔧 Clippy (1.98.0)

Clippy execution timed out

Warning

Billing warning: we have not been able to collect payment for this subscription for more than 72 hours. Please update the payment method or pay any pending invoices in Billing to avoid service interruption.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added core Core source code changes documentation Improvements or additions to documentation labels Sep 20, 2026
@DecisionNerd

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Sep 20, 2026 •

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@DecisionNerd
DecisionNerd added this pull request to the merge queue Sep 20, 2026
Merged via the queue into main with commit 097e763 Sep 20, 2026
23 checks passed
@DecisionNerd
DecisionNerd deleted the spike/1506-sort-partition-reuse branch October 1, 2026 14:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core Core source code changes documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant