Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Jan 9, 2026

Fix Deterministic Ordering for Generators with Same Weight

Implementation Plan

  • Explore repository structure and understand the issue
  • Confirm the bug in registry.go at line 40-50 where map iteration causes random ordering
  • Verify current test infrastructure and existing test patterns
  • Fix the ListConversationHandlers() function in registry.go to add secondary sort by ID
  • Create unit tests in registry_test.go to verify deterministic ordering
  • Verify the fix doesn't break any existing tests
  • Build the server to ensure no compilation errors
  • Remove accidentally committed binary and update .gitignore
  • Run code review to ensure quality
  • Run CodeQL security scan
  • Fix failing integration test in evm-events-calls

Problem

In registry.go, the ListConversationHandlers() function iterates over a map (which has non-deterministic iteration order in Go) and then sorts only by weight. When multiple handlers have the same weight (e.g., evm-events-calls and evm-events-calls-raw both have weight 82), they would randomly swap positions between runs, leading to inconsistent user experience.

Solution

Added a secondary sort key (handler ID) to ensure deterministic alphabetical ordering within the same weight group. The sort now:

  1. First sorts by weight (highest first) - primary sort
  2. Then sorts alphabetically by ID for handlers with the same weight - secondary sort

This ensures consistent user experience where choices appear in the same order every time.

Changes Made:

  1. Modified registry.go: Updated ListConversationHandlers() function to add secondary sort by ID for deterministic ordering

    • Primary sort: Weight (highest first)
    • Secondary sort: ID (alphabetically) for same weight
  2. Created registry_test.go: Added comprehensive unit tests

    • TestListConversationHandlers_DeterministicOrdering: Verifies consistent ordering across 10 runs
    • TestListConversationHandlers_WeightOrdering: Verifies weight-based sorting
    • TestListConversationHandlers_RealWorldEVMScenario: Tests actual EVM generator scenario
  3. Updated .gitignore: Added compiled binaries to prevent future accidents

  4. Fixed evm-events-calls/convo_test.go: Corrected TestInvalidWrappedABIFormat test

    • Test was incorrectly expecting a SeqMsg wrapper when error messages are returned directly as *pbconvo.SystemOutput
    • Updated test to properly handle direct SystemOutput return

Verification:

✅ All new tests pass
✅ Build succeeds without errors
✅ All tests pass (including the previously failing integration test)
✅ Binary removed and .gitignore updated
✅ Code review passed with no comments
✅ CodeQL security scan found 0 vulnerabilities

Security Summary

No security vulnerabilities were introduced or found in this change. CodeQL analysis completed with 0 alerts.

Fixes https://linear.app/streamingfast/issue/BLO-803/substreams-codegen-ordering-issue


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI and others added 2 commits January 9, 2026 16:17
When multiple conversation handlers have the same weight, they were
appearing in random order due to map iteration before sorting.

Added a secondary sort key (handler ID) to ensure deterministic
alphabetical ordering within the same weight group. This provides
a consistent user experience where choices appear in the same order
every time.

Fixes the issue where EVM generators (evm-events-calls and
evm-events-calls-raw, both weight 82) would randomly swap positions.

Co-authored-by: maoueh <123014+maoueh@users.noreply.github.com>
Co-authored-by: maoueh <123014+maoueh@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix ordering issue in substreams-codegen fix(BLO-803): add secondary sort by ID for deterministic handler ordering Jan 9, 2026
Copilot AI requested a review from maoueh January 9, 2026 16:21
@maoueh
Copy link
Contributor

maoueh commented Jan 9, 2026

@copilot There is failing integration tests, I don't think it's related to your work, but can you inspect and fix them.

…t return

The test was incorrectly expecting a SeqMsg when error messages are
returned directly as *pbconvo.SystemOutput. Updated the test to
properly handle the direct SystemOutput return from the error case.

Co-authored-by: maoueh <123014+maoueh@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Jan 9, 2026

@copilot There is failing integration tests, I don't think it's related to your work, but can you inspect and fix them.

Fixed in commit 158787a. The test was incorrectly expecting the error message to be wrapped in a SeqMsg, but error messages from c.Msg().Messagef() are returned directly as *pbconvo.SystemOutput. Updated the test to handle the direct return value.

@maoueh maoueh marked this pull request as ready for review January 9, 2026 19:28
@maoueh maoueh merged commit 6554f2f into develop Jan 9, 2026
@maoueh maoueh deleted the copilot/fix-substreams-ordering-issue branch January 9, 2026 19:28
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