Skip to content

Replace RulesEngine.logger global with task-local override + module default#6963

Merged
ajpallares merged 67 commits into
mainfrom
pallares/rules-task-local-logger
Jun 10, 2026
Merged

Replace RulesEngine.logger global with task-local override + module default#6963
ajpallares merged 67 commits into
mainfrom
pallares/rules-task-local-logger

Conversation

@ajpallares

@ajpallares ajpallares commented Jun 9, 2026

Copy link
Copy Markdown
Member

Motivation

Resolves SDK-4351. The previous RulesEngine.logger was a mutable NSLock cell whose test save/restore raced under parallel execution.

Description

  • Adds @TaskLocal scopedLogger layered over a constant default; logger is now read-only (scopedLogger ?? defaultLogger).
  • Tests use RulesEngine.$scopedLogger.withValue(...) instead of mutating shared global state.

Note

Low Risk
Logging resolution changes are localized to diagnostics; production still uses configure-time setLogger with optional per-task overrides.

Overview
Replaces mutable global test logger swapping with a @TaskLocal scopedLogger layered on the existing locked module default, so RulesEngine.logger resolves as scopedLogger ?? default without parallel tests fighting over setLogger.

RulesEngineLogger is now Sendable; LoggerStorage and test CapturingLogger adopt @unchecked Sendable for concurrency checks. setLogger remains for one-time SDK configure of the module default only.

Tests wire capture loggers via RulesEngine.$scopedLogger.withValue(...) (invokeTest in accessor tests; predicate conformance runner for warning/log fixtures) instead of save/restore around shared global state.

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

ajpallares and others added 30 commits June 2, 2026 14:56
…tures

Convert the hand-written operator unit tests for operators available at the
string+array level into declarative JSON predicate fixtures sharing khepri's
conformance format, run by a shared runner that auto-discovers every file in
PredicateFixtures/. `expected` is a khepri-compatible superset (Bool or
{ "error": ... }) plus optional `description` and `expectedWarnings`.

Adds a test-only Value: Decodable conformance so predicates and variables
decode straight into the engine's value model; production Value is unchanged.
Tests that can't be expressed as predicate->Bool stay in Swift; ValueTests
untouched. Test-only change.

Co-authored-by: Cursor <cursoragent@cursor.com>
(cherry picked from commit 40901e5)
Load the in-repo fixtures once and feed the decoded cases straight into
the parameterized test, instead of collecting IDs and re-reading every
file to find the matching case per ID. PredicateConformanceFixtureCase
gains Identifiable + Sendable (for the arguments) and a
CustomTestStringConvertible extension so each case still displays by id.

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Asserting an exact warning count coupled the fixtures to engine
internals. Match warnings by substring only (count-agnostic); an empty
`contains` now asserts that no warning is emitted, preserving the
"does not warn" cases. The three count-only fixtures gain the
"missing variable" substring they actually emit.

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Drop the khepri single-file plumbing (defaultFixtureURL, fixtureURL,
the no-arg loadCases(), the env var, and the unused fixtureNotFound
error). This PR only runs the in-repo PredicateFixtures/ directory; the
khepri conformance loader lands with its test in the conformance PR.

Co-authored-by: Cursor <cursoragent@cursor.com>
Implements the json-logic-js some/all iteration predicates. Operator coverage
is expressed as JSON predicate fixtures (some.json / all.json) run by the
shared fixture runner introduced downstack, replacing hand-written unit tests.

Co-authored-by: Cursor <cursoragent@cursor.com>
(cherry picked from commit d11529c)
Implements the json-logic-js min/max operators (variadic flat list, ±∞ for
empty input, NaN propagation for non-numeric operands). Coverage is expressed
as JSON predicate fixtures (min.json / max.json) run by the shared fixture
runner, replacing hand-written unit tests.

Co-authored-by: Cursor <cursoragent@cursor.com>
(cherry picked from commit e73eabe)
Completes the json-logic-js iteration family (none/map/filter/reduce) on top
of some/all. Coverage is expressed as JSON predicate fixtures
(none.json / map.json / filter.json / reduce.json) run by the shared fixture
runner, replacing hand-written unit tests.

Co-authored-by: Cursor <cursoragent@cursor.com>
(cherry picked from commit 46d6925)
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>

# Conflicts:
#	RevenueCat.xcodeproj/project.pbxproj
Co-authored-by: Cursor <cursoragent@cursor.com>

# Conflicts:
#	RevenueCat.xcodeproj/project.pbxproj
Co-authored-by: Cursor <cursoragent@cursor.com>

# Conflicts:
#	RevenueCat.xcodeproj/project.pbxproj
Co-authored-by: Cursor <cursoragent@cursor.com>

# Conflicts:
#	RevenueCat.xcodeproj/project.pbxproj
Expresses the "empty-string key resolves to the whole scope and is not
missing" case as a `{"!!":{"missing":[""]}}` -> false fixture; the string
coercion the other missing fixtures use can't distinguish [] from [""].
Bumps the pinned fixture count to 237. Keeps the corpus in sync with the
purchases-android counterpart.

Co-authored-by: Cursor <cursoragent@cursor.com>
Adds fixtures for the empty-segment dot-path splits, var default-vs-null-leaf
behavior, and the non-numeric missing_some threshold (7 new cases), bumping
the pinned count to 244. Also adds the var-null-path Swift test so the
non-fixturizable cases match the purchases-android counterpart. Keeps the
fixture corpus byte-identical across both repos.

Co-authored-by: Cursor <cursoragent@cursor.com>
…-logic-iteration-operators

Co-authored-by: Cursor <cursoragent@cursor.com>

# Conflicts:
#	Tests/RulesEngineInternalTests/PredicateFixtureTests.swift
…json-logic-min-max-operators

Co-authored-by: Cursor <cursoragent@cursor.com>

# Conflicts:
#	Tests/RulesEngineInternalTests/PredicateFixtureTests.swift
…on-logic-iteration-mapping-operators

Co-authored-by: Cursor <cursoragent@cursor.com>

# Conflicts:
#	Tests/RulesEngineInternalTests/PredicateFixtureTests.swift
ajpallares and others added 13 commits June 8, 2026 13:03
…ators' into pallares/json-logic-iteration-mapping-operators

Co-authored-by: Cursor <cursoragent@cursor.com>

# Conflicts:
#	Tests/RulesEngineInternalTests/PredicateFixtureTests.swift
…teration-mapping-operators

Co-authored-by: Cursor <cursoragent@cursor.com>

# Conflicts:
#	Tests/RulesEngineInternalTests/PredicateFixtureTests.swift
#	Tests/RulesEngineInternalTests/PredicateFixtures/max.json
#	Tests/RulesEngineInternalTests/PredicateFixtures/min.json
Co-authored-by: Cursor <cursoragent@cursor.com>
…ument)

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Cover logging ±Infinity, NaN, empty object, and empty array. Empty
array and empty args were indistinguishable under substring matching
(they log "" and "null"), so `expectedLogs` now asserts the exact,
ordered list of emitted messages. `expectedWarnings` is unchanged.

Co-authored-by: Cursor <cursoragent@cursor.com>
The tag was always the single `[RulesEngine]` constant and never varied
at any call site, so it added protocol surface without flexibility. For
the `log` channel a prefix also defeats the verbatim passthrough. Hosts
that bridge the logger prepend their own identifier when they need one.

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Wrap NaN- and infinity-producing subexpressions in `log` and assert the
exact emitted string via `expectedLogs`, so these fixtures verify the
intermediate value, not just the final truthiness. Covers arithmetic,
min, max, reduce, missing_some, and evaluator edge cases.

Co-authored-by: Cursor <cursoragent@cursor.com>
Place a `log` in a branch that should be skipped and assert it never
fires (`expectedLogs: []`), with positive controls that log when the
branch is evaluated — verifying and/or/if laziness.

Co-authored-by: Cursor <cursoragent@cursor.com>
… default

Layer a `@TaskLocal scopedLogger` over a write-once module default so test
logger injection is race-safe under parallel execution, and add an
`@_spi(Internal)` host-SDK wiring entry point.

Resolves SDK-4351.

Co-authored-by: Cursor <cursoragent@cursor.com>
Defer the host-SDK wiring entry point to a follow-up. This keeps the change
to the internal `@TaskLocal scopedLogger` test mechanism layered over a
constant default logger.

Co-authored-by: Cursor <cursoragent@cursor.com>
Base automatically changed from pallares/ios-rules-fixture-log-assertions to main June 9, 2026 12:33
…ocal-logger

Co-authored-by: Cursor <cursoragent@cursor.com>

# Conflicts:
#	RulesEngineInternal/Logger.swift
#	Tests/RulesEngineInternalTests/Helpers/CapturingLogger.swift
#	Tests/RulesEngineInternalTests/Helpers/PredicateConformanceRunner.swift
#	Tests/RulesEngineInternalTests/PredicateFixtures/log.json

/// Logging facade for the rules engine.
protocol RulesEngineLogger {
protocol RulesEngineLogger: Sendable {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Sendable conformance is required by @TaskLocal

@ajpallares ajpallares marked this pull request as ready for review June 9, 2026 13:02
@ajpallares ajpallares requested a review from a team as a code owner June 9, 2026 13:02
@ajpallares ajpallares requested a review from a team June 10, 2026 10:54
ajpallares and others added 2 commits June 10, 2026 12:54
Brings back the lock-guarded `LoggerStorage` so the default logger can be
replaced (host-SDK wiring will expose this in a follow-up PR), while keeping
everything internal to the module.

Co-authored-by: Cursor <cursoragent@cursor.com>

@rickvdl rickvdl left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice, clever solution 💡

@ajpallares

Copy link
Copy Markdown
Member Author

@RCGitBot please test

@ajpallares ajpallares merged commit 00ed9b4 into main Jun 10, 2026
43 checks passed
@ajpallares ajpallares deleted the pallares/rules-task-local-logger branch June 10, 2026 15:49
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