Skip to content

fix: update cached impression timestamps after repeated evaluations - #661

Draft
Kayvan-Zahiri wants to merge 1 commit into
splitio:developmentfrom
Kayvan-Zahiri:codex/update-cached-impression-timestamps
Draft

Kayvan-Zahiri wants to merge 1 commit into
splitio:developmentfrom
Kayvan-Zahiri:codex/update-cached-impression-timestamps

Conversation

@Kayvan-Zahiri

Copy link
Copy Markdown

Updating an existing SimpleLruCache key returns its old value but never stores the replacement. Consequently, Observer keeps reporting the first timestamp for an impression, and optimized mode continues queueing duplicates after crossing an hour boundary: the cached timestamp remains in the earlier hour.

Assign the new value to the existing node while retaining the previous value for the return. LRU promotion and eviction remain unchanged.

Four new regressions cover repeated replacement, replacement plus eviction, observer previous timestamps, and optimized deduplication after an hour boundary. All four fail before the fix. Existing manager/listener expectations are updated to use the immediately preceding timestamp instead of the first timestamp.

Validation on Python 3.13 with the declared test dependencies:

  • All 127 engine tests pass, including the existing cache controls and four new regressions.
  • Broader offline selection: 398 pass and two Redis factory failures. Both failures reproduce on an untouched archive of the development branch in the same environment.
  • python setup.py build, changed-source Flake8, and git diff --check pass.
  • Full-source Flake8 produces 1,791 findings; its output is byte-for-byte identical on the untouched baseline.

Full-suite verification is incomplete: a restricted run stopped at 318 passes, the two baseline factory failures, and three local-server socket-permission errors. A retry with localhost access excluding service integration/Redis storage tests stopped progressing in factory tests and was interrupted. Redis service integration and the supported Python/platform matrix remain unverified. No hosted Harness/Split requests or credentials were used.

Targets development as required by CONTRIBUTORS-GUIDE.md. Developed with Codex assistance and reproduced locally.

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.

1 participant