Skip to content

perf(exec): serial disposition for maximum_spanning_tree (#580) - #678

Merged
DecisionNerd merged 7 commits into
mainfrom
cursor/580-serial-maximum-spanning-tree-disposition-c6a4
Aug 12, 2026
Merged

DecisionNerd merged 7 commits into
mainfrom
cursor/580-serial-maximum-spanning-tree-disposition-c6a4

Conversation

@DecisionNerd

@DecisionNerd DecisionNerd commented Aug 10, 2026 •

Copy link
Copy Markdown
Contributor

Implements #580: serial disposition for maximum_spanning_tree.

Closes #580

Open in Web Open in Cursor 

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

Note

Add serial disposition for analyze(by=MaximumSpanningTree) with test coverage

  • Adds serial_kruskal_descending_union_find as the fixed disposition for GraphForge::analyze with MaximumSpanningTree, with no parallel crossover regardless of compute_threads.
  • Adds a run_analyze_maximum_spanning_tree test helper that exercises the analyze call, checks determinism across two invocations, and records WorkloadEvidence with the new disposition.
  • Extends the M4 entry matrix contract and CI validation script to require the analyze-maximum-spanning-tree workload and validate it against structural gates.
  • Documents the serial-only policy in execution-resource-policy.md.

Macroscope summarized ceb8dc4.

Summary by CodeRabbit

  • Tests

    • Added coverage for maximum-spanning-tree analysis using deterministic, cost-weighted graph data.
    • Added repeatability checks for results, output fingerprints, ordering, row counts, and bounded output.
    • Included the workload in short and large execution matrices with validation of serial processing behavior.
  • Documentation

    • Documented why spanning-forest analysis uses serial processing to preserve consistent edge-selection results.

@coderabbitai

coderabbitai Bot commented Aug 10, 2026 •

Copy link
Copy Markdown

Review Change Stack

Walkthrough

Added deterministic maximum-spanning-tree analysis coverage to the M4 baseline and entry matrix. The workload compares repeated results, fingerprints output, records timing and RSS, and documents serial union-find execution.

Changes

Maximum-spanning-tree M4 coverage

Layer / File(s) Summary
Maximum-spanning-tree workload evidence
crates/graphforge-api/tests/m4_entry_baseline.rs, crates/graphforge-exec/src/algorithm_analyze_minimum_spanning_forest.rs
The baseline runs GraphForge::analyze twice, compares results, fingerprints output, records timing and RSS, and reports serial Kruskal/union-find metadata.
M4 matrix contract integration
tests/contracts/m4-entry-matrix.json, scripts/ci/m4-entry-matrix.py
The entry matrix defines maximum-spanning-tree execution and structural gates. CI validation now requires the workload ID.

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

Possibly related issues

Possibly related PRs

Suggested labels: ci-cd

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description states the issue and summarizes the implementation, but it omits most required template sections, testing details, checklist items, and reviewer notes. Complete the required description sections, including change type, testing commands and coverage, checklist confirmations, performance impact, breaking changes, and reviewer notes.
Linked Issues check ❓ Inconclusive The changes support the serial disposition, deterministic workload evidence, and CSR/Arrow CI gates, but cancellation, limits, structured errors, documentation, and CI results are not fully verifiable. Provide evidence for cancellation, limits, structured errors, exact-head CI, and the excluded execution-resource policy documentation.
✅ Passed checks (3 passed)
Check name Status Explanation
Out of Scope Changes check ✅ Passed All reviewed changes directly support the linked issue by documenting, testing, and enforcing the maximum spanning tree serial disposition.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Title check ✅ Passed The title clearly identifies the main change: a serial execution disposition for maximum spanning tree analysis.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch cursor/580-serial-maximum-spanning-tree-disposition-c6a4

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

@github-actions github-actions Bot added executor Changes to query executor core Core source code changes testing Test coverage and testing infrastructure documentation Improvements or additions to documentation tooling Developer tooling and automation labels Aug 10, 2026
@DecisionNerd
DecisionNerd force-pushed the cursor/580-serial-maximum-spanning-tree-disposition-c6a4 branch from 7b5adf4 to a9ae051 Compare August 11, 2026 23:04
@DecisionNerd
DecisionNerd marked this pull request as ready for review August 11, 2026 23:06
@DecisionNerd
DecisionNerd force-pushed the cursor/580-serial-maximum-spanning-tree-disposition-c6a4 branch 2 times, most recently from bdb3498 to 4ead674 Compare August 12, 2026 01:09

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/graphforge-api/tests/m4_entry_baseline.rs`:
- Around line 606-620: Replace the hard-coded execution fields in the baseline’s
structural evidence with values observed from the GraphForge::analyze algorithm
path. Instrument or expose assertions for the actual thread policy, CSR
projection, and bounded Arrow sink behavior, and populate threads_path,
csr_native_projection, and bounded_arrow_sink from those observations rather
than constants.
- Around line 593-603: Add a fixture-specific correctness assertion in the
maximum_spanning_tree test before constructing WorkloadEvidence, validating the
expected selected edge endpoints and weights (or comparing against the approved
fixture fingerprint). Keep the existing assert_logical_batch_equal check to
preserve determinism coverage.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 6fc9c8ef-17fb-46db-b3f7-7388c025273b

📥 Commits

Reviewing files that changed from the base of the PR and between 26fb547 and 4ead674.

⛔ Files ignored due to path filters (1)
  • docs/development/execution-resource-policy.md is excluded by !**/*.md, !**/docs/**
📒 Files selected for processing (4)
  • crates/graphforge-api/tests/m4_entry_baseline.rs
  • crates/graphforge-exec/src/algorithm_analyze_minimum_spanning_forest.rs
  • scripts/ci/m4-entry-matrix.py
  • tests/contracts/m4-entry-matrix.json

Comment thread crates/graphforge-api/tests/m4_entry_baseline.rs
Comment thread crates/graphforge-api/tests/m4_entry_baseline.rs
@blacksmith-sh

This comment has been minimized.

@DecisionNerd
DecisionNerd force-pushed the cursor/580-serial-maximum-spanning-tree-disposition-c6a4 branch 2 times, most recently from 16bca2c to b8419b8 Compare August 12, 2026 01:50
@DecisionNerd
DecisionNerd force-pushed the cursor/580-serial-maximum-spanning-tree-disposition-c6a4 branch from b8419b8 to 1274921 Compare August 12, 2026 02:14
@blacksmith-sh

This comment has been minimized.

@DecisionNerd
DecisionNerd force-pushed the cursor/580-serial-maximum-spanning-tree-disposition-c6a4 branch 5 times, most recently from 4486466 to 19fcd01 Compare August 12, 2026 03:07
@blacksmith-sh

This comment has been minimized.

cursoragent and others added 6 commits August 11, 2026 21:17
Closes #580

Co-authored-by: David Spencer <DecisionNerd@users.noreply.github.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@DecisionNerd
DecisionNerd force-pushed the cursor/580-serial-maximum-spanning-tree-disposition-c6a4 branch from 6e52567 to ceb8dc4 Compare August 12, 2026 03:18
@DecisionNerd
DecisionNerd merged commit d9870ca into main Aug 12, 2026
21 checks passed
@DecisionNerd
DecisionNerd deleted the cursor/580-serial-maximum-spanning-tree-disposition-c6a4 branch August 12, 2026 03:26
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 executor Changes to query executor testing Test coverage and testing infrastructure tooling Developer tooling and automation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

perf(exec): polish scale/performance for analyze(by="maximum_spanning_tree")

2 participants