Skip to content

Ensure common aggregations run before other aggregations#651

Merged
SandeepTuniki merged 4 commits into
masterfrom
fix-aggregation-execution-ordering
Jul 16, 2026
Merged

Ensure common aggregations run before other aggregations#651
SandeepTuniki merged 4 commits into
masterfrom
fix-aggregation-execution-ordering

Conversation

@SandeepTuniki

@SandeepTuniki SandeepTuniki commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

In the present code, there's a danger that before common aggregations (ex: linked edges, provenance summary) can execute, other aggregations dependent on it (ex: places) might run first.

Right now, the order is correctly resolved due to naming of yaml files, but this is brittle and can break anytime the file names in config are changed. To prevent it, I'm enforcing a priority order explicitly.

@codacy-production

codacy-production Bot commented Jul 16, 2026

Copy link
Copy Markdown

Not up to standards ⛔

🔴 Issues 2 minor

Alerts:
⚠ 2 issues (≤ 0 issues of at least minor severity)

Results:
2 new issues

Category Results
Documentation 2 minor

View in Codacy

🟢 Metrics 6 complexity

Metric Results
Complexity 6

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

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

Copy link
Copy Markdown
Contributor

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 introduces deterministic execution priority sorting for calculation types within a stage. It defines priority ranks (CALCULATION_TYPE_PRIORITY) in common.py, updates the orchestrator to sort calculations by stage and priority, moves several prerequisite calculations to stage 0 in the configuration, and updates the JSON schema to require input_imports for all calculation types except EMBEDDING_GENERATION. Unit tests are added to verify the ordering logic. The reviewer suggested adding a sanity test to ensure all CalculationType enum members have a defined priority in CALCULATION_TYPE_PRIORITY to prevent future bugs.

@SandeepTuniki
SandeepTuniki marked this pull request as ready for review July 16, 2026 13:35
@SandeepTuniki
SandeepTuniki enabled auto-merge (squash) July 16, 2026 13:43
@SandeepTuniki
SandeepTuniki merged commit 5ef5144 into master Jul 16, 2026
10 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants