Skip to content

fix: prevent duplicate suite evaluation in Mobilize#186

Merged
jmeridth merged 1 commit intomainfrom
fix/mobilize-dedup-and-break
Mar 30, 2026
Merged

fix: prevent duplicate suite evaluation in Mobilize#186
jmeridth merged 1 commit intomainfrom
fix/mobilize-dedup-and-break

Conversation

@jmeridth
Copy link
Copy Markdown
Member

What

Add a dedup guard in addEvaluationSuite to silently skip duplicate CatalogId registrations, and add a break in Mobilize's inner loop to stop after the first matching suite per catalog.

Why

The private addEvaluationSuite method appended unconditionally, so repeated AddEvaluationSuite calls with the same catalog ID would register duplicate suites. The Mobilize inner loop then evaluated and appended all of them, causing duplicate results.

Notes

  • Both issues are defense-in-depth since the public API guards against duplicates via referenceCatalogs uniqueness, but direct construction (e.g. in tests) could trigger the duplicate path
  • The dedup guard in addEvaluationSuite uses a linear scan, which is fine given the small number of suites in practice
  • The break in Mobilize enforces one-suite-per-catalog-ID semantics; if multi-suite-per-catalog is ever needed, this would need revisiting

Testing

  • Added TestEvaluationOrchestrator_AddEvaluationSuite_DeduplicatesCatalogId: calls AddEvaluationSuite twice with the same catalog ID, verifies only one suite is registered
  • Added TestEvaluationOrchestrator_Mobilize_BreaksAfterMatch: manually constructs two suites with the same CatalogId, verifies Mobilize only evaluates and appends the first one
  • Full test suite passes (go vet ./..., go test ./..., golangci-lint run ./...)

## What

Add a dedup guard in addEvaluationSuite to silently skip duplicate
CatalogId registrations, and add a break in Mobilize's inner loop to
stop after the first matching suite per catalog.

## Why

The private addEvaluationSuite method appended unconditionally, so
repeated AddEvaluationSuite calls with the same catalog ID would
register duplicate suites. The Mobilize inner loop then evaluated and
appended all of them, causing duplicate results.

## Notes

- Both issues are defense-in-depth since the public API guards against
  duplicates via referenceCatalogs uniqueness, but direct construction
  (e.g. in tests) could trigger the duplicate path
- The dedup guard in addEvaluationSuite uses a linear scan, which is
  fine given the small number of suites in practice
- The break in Mobilize enforces one-suite-per-catalog-ID semantics;
  if multi-suite-per-catalog is ever needed, this would need revisiting

Signed-off-by: jmeridth <jmeridth@gmail.com>
@jmeridth jmeridth self-assigned this Mar 30, 2026
@github-actions github-actions bot added the fix label Mar 30, 2026
@jmeridth jmeridth marked this pull request as ready for review March 30, 2026 23:32
@jmeridth jmeridth requested a review from a team as a code owner March 30, 2026 23:32
@jmeridth jmeridth merged commit 7b74cbb into main Mar 30, 2026
9 checks passed
@jmeridth jmeridth deleted the fix/mobilize-dedup-and-break branch March 30, 2026 23:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants