Skip to content

Fix aggregate workflow query assertion - #358

Closed
delkc wants to merge 1 commit into
mainfrom
clay/fix-workflow-batch-test
Closed

delkc wants to merge 1 commit into
mainfrom
clay/fix-workflow-batch-test

Conversation

@delkc

@delkc delkc commented Sep 28, 2026

Copy link
Copy Markdown
Contributor

Why

PR #326 added a relay-discovery test that expects one workflow filter per channel. Production and the existing workflow pagination tests use one aggregate filter with up to 128 sorted channel IDs, so the new assertion fails on main and blocks unrelated pull requests.

What

Update the assertion to match the aggregate workflow query contract.

How

The test now checks one kind 30620 filter with the sorted channel batch in #h and the existing definition limit.

Risk

Low. This changes one test assertion and no production behavior.

Testing

bin/pnpm exec vitest run src/features/relay/store-discovery.test.ts --testNamePattern 'discovers and names 501 same-timestamp memberships'

The pre-push hook also passed all 27 related tests and design-system checks.

Generated with Goose

Signed-off-by: Clay Delk <clay.delk@gmail.com>
@delkc
delkc marked this pull request as ready for review September 28, 2026 18:56
@delkc
delkc requested review from a team, comp615 and wesbillman as code owners September 28, 2026 18:56

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Star Lord automated source review (via Wes’s account)

No actionable findings in this assertion-only fix. Reviewed head 43b91ad08db937d1ab4a720bb6d23d4ad4753889 against base/merge base 7834fffa365e019fdc4758453275a826095a0725.

The exact single-filter assertion matches createWorkflows().definitions(batch) (src/features/workflows/capability.ts:387–421): one kind-30620 filter containing the batch, with definition limit 100. The real relay reader canonicalizes tag arrays with deduplication and sorting (src/features/relay/reader.ts:194–235), so [...batch].sort() is appropriate at the scripted transport boundary. This also agrees with docs/workflows.md:74–84 and the existing aggregate/cursor regressions in src/features/workflows/pagination.test.ts:53–92. The 501-membership fixture, second-page workflow result assertion, completion wait and view disposal remain intact; no production behavior, test cases or pagination checks were removed.

Validation boundary: source-only comparison of the complete one-file diff and relevant production/test paths; no tests, builds, installs, app launches or PR code executed. The author reports a focused test and pre-push checks; I did not independently execute them. One hosted CI snapshot showed JavaScript, Rust/tool integration and all six browser journey shards still running; browser measurements, security checks and DCO had succeeded, and Windows was skipped. No CI polling or claim of a fully green suite. Live relay, packaged-native and human acceptance remain unverified. This is a non-blocking COMMENT review, not approval or merge authorization.

@delkc

delkc commented Sep 28, 2026

Copy link
Copy Markdown
Contributor Author

Closing this separate PR. The one-line merged-tree compatibility assertion is being kept with the original Membership PR so this remains one review unit.

@delkc delkc closed this Sep 28, 2026
@delkc
delkc deleted the clay/fix-workflow-batch-test branch September 28, 2026 19:30
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