Skip to content

chore: Deduplicate shared sync/async evaluator target checking - #482

Merged
jsonbailey merged 2 commits into
mainfrom
jb/sdk-2871/async-dedup
Aug 11, 2026
Merged

chore: Deduplicate shared sync/async evaluator target checking#482
jsonbailey merged 2 commits into
mainfrom
jb/sdk-2871/async-dedup

Conversation

@jsonbailey

@jsonbaileyjsonbailey commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Overview

Behavior-preserving cleanup extracted from the async SDK implementation branch (epic SDK-60). Touches only files already on main; independent of the async-client / FDv2 work.

Track Internally: SDK-2871

Changes

Evaluator dedup (core):

  • ldclient/impl/evaluator_common.py — add a shared check_targets() free function holding the context/user target-matching logic.
  • ldclient/impl/evaluator.py — remove the private _check_targets method; route to the shared check_targets().
  • ldclient/impl/async_evaluator.py — same: remove the duplicated _check_targets method and route to the shared function.

The sync and async evaluators previously carried byte-identical copies of the target-matching logic. This collapses them into one implementation.

Async test cleanup:

  • ldclient/testing/impl/test_async_evaluator.py, test_async_big_segments.py, test_async_flag_tracker.py, test_async_hooks.py, integrations/test_async_redis.py — drop redundant comment-banner section headers so the async test modules match the sync test style. No test logic changes.

Verification

  • uv run pytest on the evaluator and async component test files: all pass (async redis suite verified green against a local redis; it connects to a live server as in CI).
  • uv run pycodestyle, uv run isort --check --atomic, uv run mypy clean on all changed files.

No CHANGELOG or version changes.


Note

Overview
Target matching for user and context targets is consolidated into a shared check_targets() in evaluator_common.py. Both Evaluator and AsyncEvaluator now call that function instead of maintaining identical private _check_targets methods.

Async test modules lose decorative section-header comment banners (and one hook-test helper docstring is clarified); no test behavior changes.

Reviewed by Cursor Bugbot for commit e99e817. Bugbot is set up for automated code reviews on this repo. Configure here.

Move the target-matching logic into a shared check_targets() free
function in evaluator_common, and route both the sync and async
evaluators to it. Also drop redundant comment-banner headers from
several async test modules to match the sync test style.
Behavior is unchanged.
SDK-2871
@jsonbailey
jsonbailey requested a review from a team as a code ownerAugust 5, 2026 22:28
@jsonbaileyjsonbailey changed the title refactor: Deduplicate shared sync/async evaluator target checkingchore: Deduplicate shared sync/async evaluator target checkingAug 5, 2026

@joker23joker23 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.

nit comments only

Comment threadldclient/impl/async_evaluator.py Outdated
Comment threadldclient/impl/evaluator.py
@jsonbailey
jsonbailey merged commit bc0d06e into mainAug 11, 2026
15 checks passed
@jsonbailey
jsonbailey deleted the jb/sdk-2871/async-dedup branch August 11, 2026 19:10
Sign up for freeto 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

@jsonbailey@joker23