Skip to content

feat: Add the override marker to the models and mark evaluations - #526

Merged
kinyoklion merged 14 commits into
feat/overridesfrom
rlamb/overrides-python-model-marker
Nov 6, 2026
Merged

kinyoklion merged 14 commits into
feat/overridesfrom
rlamb/overrides-python-model-marker

Conversation

@kinyoklion

@kinyoklion kinyoklion commented Sep 25, 2026 •

Copy link
Copy Markdown
Member

Summary

This is the second step of the flag overrides port described by the OVERRIDE specification. It is based on the shared file data branch because the phases are stacked; retarget to feat/overrides once that branch merges.

The FeatureFlag and Segment models gain an override marker, is_override. The SDK represents stored entities with typed model classes distinct from their JSON form, so the marker is carried on the type and never in the JSON: to_json_dict(), the data kind encoder, and ModelEncoder produce the same output for a marked and an unmarked entity, and a persistent store never sees it. with_override_marker() returns a marked shallow copy that shares the definition and leaves the source unmarked, so a source can retain and resupply its entities. Only the SDK components that manage override entries (added in the next step) set the marker.

The evaluator marks an evaluation as override-affected when any definition it reads carries the marker: the evaluated flag, a prerequisite flag at any depth, or a segment consulted during clause matching. A read counts whether or not the segment matches, including through a negated clause and through a segment referenced by another segment's rule. A definition that cannot be resolved contributes nothing, because nothing was read. The marking propagates upward only: a prerequisite is an evaluation in its own right, its record reflects only the definitions its own subtree read, so an unaffected prerequisite inside a marked evaluation stays unmarked and a marked prerequisite does not mark a sibling. An evaluation that fails with an error reason, including a prerequisite cycle, is still marked when it read an override definition.

The marking is reported in two places that always agree: the overrideAffected indicator on the evaluation reason, present only when true so an unaffected reason serializes exactly as before, and EvalResult.override_affected, which the client and event processing read in the following steps. The AsyncEvaluator receives the same changes.

Tests cover each marking condition for both evaluators (flag off, fallthrough, and rule match; plain evaluations; malformed flag and prerequisite cycle errors; prerequisite records at depth one and two; unaffected sibling and downward non-propagation; segment reads with and without a match, negated, nested, and through a prerequisite; missing definitions; scalar and indicator agreement) and the model marker (default, marked copy, never serialized, shared definition).

SDK-3250


Note

Overview
Adds experimental flag override tracking: FeatureFlag and Segment now expose is_override and with_override_marker() on in-memory models only—the marker never appears in JSON or persisted data.

Sync and async evaluators set EvalResult.override_affected and, when true, add overrideAffected to the evaluation reason (documented on EvaluationDetail.reason). An evaluation is marked when the root flag, any prerequisite flag read during evaluation, or any segment loaded for rule matching carries the override marker; unresolved flags/segments do not count, and segment reads count even when the segment does not match (including negated clauses and nested segment rules). Prerequisite evaluation uses scoped marking that merges upward so sibling prereq events stay unmarked while affected subtrees and the top-level result reflect overrides; error outcomes are still marked if an override definition was read.

Extensive tests cover model serialization, marking rules, and parity between sync and async evaluators.

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

@kinyoklion
kinyoklion force-pushed the rlamb/overrides-python-model-marker branch from 60d6f22 to 34322d8 Compare September 25, 2026 22:11
@kinyoklion
kinyoklion force-pushed the rlamb/overrides-python-filedata-reloader branch from c478bd1 to 7e64242 Compare September 28, 2026 20:28
@kinyoklion
kinyoklion force-pushed the rlamb/overrides-python-model-marker branch from 34322d8 to ee313c1 Compare September 28, 2026 20:28
Adds ldclient.impl.integrations.files.filedata, the file reading, parsing,
and merging logic that the file-based override source is built on. A
document that starts with an opening brace is parsed as JSON and any other
document as YAML. Definitions are decoded into the flag and segment models
while the file is read, so an invalid definition fails that load. Files are
merged in the configured order with a duplicate keys handling of fail or
ignore, and the result records how many entries each file supplied.

The Reloader owns the reload cycle: it serializes reloads, debounces change
signals with a settle window, keeps the last good data by not applying a
failed load, retries a failed load after a bounded delay, reports an identical
failure once, and skips an application whose file contents did not change. It
can treat a configured file that does not exist as a file with no content.
The Poller detects changes by comparing modification time and size on an
interval, including files that appear or disappear. The Watcher uses the
watchdog package, watches the directory of each file so an absent file is
picked up when it appears, matches the destination of a move so a file
written by rename is detected, retries a directory that does not exist yet,
and reacts only to notifications that can change a file's content or
presence.

The existing file data sources are not changed and keep their current
behavior. A new test module pins that behavior: the flagValues expansion and
its evaluation reason, the version fallback, the failure messages, the FDv2
status and error kinds, and the polling and watching rules.
@kinyoklion
kinyoklion force-pushed the rlamb/overrides-python-filedata-reloader branch from 7e64242 to ee3234c Compare September 30, 2026 20:31
@kinyoklion
kinyoklion force-pushed the rlamb/overrides-python-model-marker branch from ee313c1 to 2c70075 Compare September 30, 2026 20:31
@kinyoklion
kinyoklion force-pushed the rlamb/overrides-python-model-marker branch from 2c70075 to 5f738a6 Compare October 1, 2026 23:43
…st watch

The reloader treats an exception from apply like a load failure: it is reported, retried, and the result is not remembered as the last good one. An exception from the error callback is logged and does not stop the retry. The watcher retries when the observer cannot start, watches a directory again after it is deleted and recreated, and closes without error when the observer never started.
@kinyoklion
kinyoklion force-pushed the rlamb/overrides-python-model-marker branch from 5f738a6 to 0013c0e Compare October 3, 2026 01:18
@kinyoklion

Copy link
Copy Markdown
Member Author

bugbot review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 0013c0e. Configure here.

A plain rewrite is not atomic: the truncation and the write are separate steps, and a poll that lands between them legitimately reports an extra change. The poller promises generous detection, not one callback per logical change; the reloader coalesces and skips the excess. The test that expected exactly one callback could therefore fail on a busy machine. It now drains the callbacks of the write and then requires silence, which still fails for a poller that does not update its baseline.
@kinyoklion
kinyoklion force-pushed the rlamb/overrides-python-model-marker branch from 0013c0e to 85a71df Compare October 13, 2026 21:02
…failure is reported

The reloader test wrote a file with a new failure while the automatic retries were running and
expected the next retry to report it. The write is ordinary, not atomic, so a retry can read the
file while it is empty or half written. An empty document is valid, so such a retry succeeds and
ends the retries, and the new failure is never read; a half-written file is reported as a failure
of its own. The test now loads the final content on demand and inspects the reports as a whole,
and a fixture helper collects the callbacks that arrive within a settle period.
@kinyoklion
kinyoklion force-pushed the rlamb/overrides-python-model-marker branch from 85a71df to f3db8c6 Compare October 13, 2026 22:40
@kinyoklion
kinyoklion marked this pull request as ready for review November 2, 2026 23:02
@kinyoklion
kinyoklion requested a review from a team as a code owner November 2, 2026 23:02
…y extension

Applications and editors rewrite data files in place: they truncate the file
and then write the new content. With pyyaml installed, a load in between parsed
the empty file as a document with no entries, whatever its name.

The parser is now chosen by the file name: a name ending in .yaml or .yml is
YAML, and any other file is JSON. Files must be UTF-8. An empty JSON file, and
JSON that is not an object such as null, fail to parse; YAML that is empty,
only comments, or null has no entries.

The Reloader holds back a file whose content is in effect and that a load finds
with no entries, until it has stayed so, unwritten, for the retry delay. The
delay is measured from the file's own read and starts over when the file's
modification time or size changes. Each file is held for at most five retry
delays from its first hold, and its bound starts over only at a load that
holds nothing back. A load reads and classifies every file, even after a
failure, and fails with the first failure; content, even content that cannot
be parsed, or a missing file ends the empty state. The skip-unchanged digest
covers each path, whether its file exists, and its bytes.

Tests rewrite files in place through ldclient/testing/file_util.py and time
the hold on an injected clock.
An entry's own key was kept when it differed from its map key. The entry was stored under one key and reported by evaluations under the other, and two entries with the same key under different map keys were two items with one key. The map key is now the entry's key for every purpose.
The watcher matched events against the configured paths in their real directories, so an edit to the target of a configured symbolic link in another directory was never reported. The watcher now also watches the directory of each link's target for the target's path, and resolves the links again after every reported change, after a retry sets up watches, and when a directory that holds a target is deleted, as a ConfigMap update does.
@kinyoklion
kinyoklion force-pushed the rlamb/overrides-python-model-marker branch from d9973c8 to 02a32ab Compare November 4, 2026 19:20
…tched

A link whose target's directory did not exist when the links were resolved, at start or after the directory was deleted, was not resolved again until another change was reported, so edits to its target after the directory appeared were missed. Such a link, or a target directory that cannot be watched, now keeps the retry schedule running, which resolves the links again, and the callback runs once when a retry watches a target. A relative link is now joined to the real path of the directory that holds it, which a test pins for a link in a directory reached through another link.
@kinyoklion
kinyoklion force-pushed the rlamb/overrides-python-model-marker branch from 02a32ab to 6393a98 Compare November 4, 2026 19:49
…atched directory, and warn once for an unwatchable directory

Two threads that resolved the links at once could both find a target's directory unwatched; the observer shares one watch per directory, so the thread that lost the race removed the watch the other had stored, and the target was never watched again. That watch is now kept. A retry that resolved a link to a target in a directory that was already watched set up no watch and so signaled no change; it now signals whenever a link leads to a new target. The retry logged a warning for a directory that cannot be watched yet on every attempt; it now warns once per directory and reason and logs repeats at debug level until the directory is watched. The watcher docstring now names the changes that watching does not follow, a link further along the chain or a configured directory that is pointed elsewhere or replaced, and points such deployments to polling.
@kinyoklion
kinyoklion force-pushed the rlamb/overrides-python-model-marker branch from 6393a98 to 29b98ac Compare November 4, 2026 22:55
…ectory's warning only where the loss is reported

Windows needs to know that a link leads to a directory when the link is created, so the watcher tests now say so for their directory links. The race test reads the target directory's key from the watcher instead of computing it, since Windows resolves the path differently. The loss of a watched directory is reported through inotify, so the part of the warning test that loses the directory runs on Linux only.
Adds an override marker to the FeatureFlag and Segment models. The marker is
an attribute of the model type, never part of the JSON representation, so
serializing an entity or writing it to a persistent store never carries it.
A marked shallow copy is produced with with_override_marker(), which leaves
the source entity unmarked and shares its definition.

The evaluator marks an evaluation as override-affected when any definition
it reads carries the marker: the evaluated flag, a prerequisite at any depth,
or a segment consulted during rule matching, including a segment that does
not match and a segment referenced by another segment. The marking propagates
upward only. A prerequisite's own record reflects the definitions its subtree
read, so an unaffected prerequisite inside a marked evaluation is not marked
and a marked prerequisite does not mark a sibling. An evaluation that fails
with an error reason is still marked when it read an override definition.

The marking appears on the evaluation reason as the overrideAffected
indicator, present only when true, and on EvalResult.override_affected for
the client and event processing. The async evaluator mirrors the sync one.
@kinyoklion
kinyoklion force-pushed the rlamb/overrides-python-model-marker branch from 29b98ac to 1debdc2 Compare November 4, 2026 23:09
Base automatically changed from rlamb/overrides-python-filedata-reloader to feat/overrides November 6, 2026 17:42
@kinyoklion
kinyoklion merged commit 8c5131c into feat/overrides Nov 6, 2026
7 checks passed
@kinyoklion
kinyoklion deleted the rlamb/overrides-python-model-marker branch November 6, 2026 17:42
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