Conversation
Two manager-context tests carried 2026-09-13 timestamps while retention drops material older than seven days measured against the wall clock, so both turned red on their own seven days later and took every open pull request with them. CI brackets the boundary: the 05:03Z main run passed this file, the 06:43Z main run failed it, with no merge in between. The helper resolves to whole minutes so the captured material and the expected payload still compare equal, and the context keeps preceding the turn that consumes it. Production code is untouched. Signed-off-by: DJC1412 <108855841+DJC1412@users.noreply.github.com>
huangruiteng
left a comment
There was a problem hiding this comment.
Review of exact head 9993fc511fc6eb19b82b0d516a0e1813f5072afc
[P1] Anchor the relative clock once
_fixture_time() calls datetime.now(UTC) on every use, while both affected tests call it separately when constructing provider events and when constructing the expected context_materials. If execution crosses a whole-minute boundary between those calls, the same minutes_ago value resolves one minute apart and the equality assertion fails. That preserves a lower-frequency wall-clock flake in the test intended to remove the dated-fixture flake.
Please capture one whole-minute anchor once, either at module/test setup or in each affected test, and derive every related timestamp from that same anchor. A module-level _FIXTURE_NOW = datetime.now(UTC).replace(second=0, microsecond=0) is sufficient here because the material remains far inside the seven-day retention boundary.
Validation: the full test file passes locally (55 passed), which confirms the ordinary path; the blocker is the independently visible boundary transition between repeated clock reads. I found no production behavior, authority, privacy, or compatibility change in this test-only diff.
English verdict: REQUEST CHANGES - exact head 9993fc511fc6eb19b82b0d516a0e1813f5072afc; use one shared clock anchor so the regression test cannot fail at a minute boundary.
|
Superseded by #4793, which merged the shared fixed-clock repair for the two dated manager-context fixtures together with the module-budget repair. The current main merge of #4802 includes that fix, and the three formerly failing post-merge checks now pass locally. Closing this alternate dynamic-time fixture approach so the repository keeps one canonical repair. |
Goal And Delivered Outcome
2026-09-13T05:59/06:00Zevent dates whileMANAGER_CONTEXT_MAX_AGE(loopx/extensions/lark/manager_context.py:28) drops material older than seven days measured againstdatetime.now(UTC). The fixtures therefore expired on 2026-09-20T05:59Z and have failed every branch since, includingmainitself.pytest -q tests/extensions/test_lark_goal_topic_runtime.pyon a cleanorigin/mainworktree gave2 failed, 49 passed, both failuresassert 0 == 1oncontext_material_count; after, the same command gives 55 passed and keeps passing whenever it runs. Proven by theunitandregression_parityrows.main.Scope And Continuation
_fixture_time(minutes_ago), replaces the seven literals at 60/61 minutes ago, which preserves the ordering the tests rely on (context precedes the turn that consumes it) and resolves to whole minutes so the captured material and the expected dictionary still compare equal — the second symptom in [Bug]: absolute fixture dates make the Lark manager-context tests expire after seven days #4797 was exactly that mismatch reproducing with different microseconds. Not claimed: a direct test of theretention_expiredbranch. Reaching it through the shipped path means capturing material older than the collector's own seven-day lookback, so such a test would pass for a second reason and certify nothing; pinning the rule properly needs an injectednowat theprocess_lark_goal_topic_eventboundary, which is a production-signature decision rather than a fixture fix.now-injection question stays with whoever owns the Lark manager adapter, noted in [Bug]: absolute fixture dates make the Lark manager-context tests expire after seven days #4797.Validation
9993fc511unituv run --extra test python -m pytest -q tests/extensions/test_lark_goal_topic_runtime.py→ 55 passed; the two previously failing tests are the ones that now pass.integrationuv run --extra test python -m pytest -q tests/extensions→ 943 passed, no failures or skips introduced.regression_parityorigin/mainworktree with this diff absent:2 failed, 49 passed, withAssertionError: assert 0 == 1at line 395/557 for the two named tests. Repository CI brackets the same expiry without any merge in between — run35490703890(main361347713, 05:03Z) reports 0 failures in this file across all four shards, run35494985951(main4eb6ea9ee, 06:43Z) reports both of them failing, either side of2026-09-13T05:59Z + 7 days = 2026-09-20T05:59Z.staticpython -m py_compileon the changed file;loopx check --scan-path tests/extensions/test_lark_goal_topic_runtime.py→ public boundary scan clean.tests/extensionssuite is the covering surface. Not run:npm run test:control-plane(no TypeScript touched), and the pre-existingtests/canary/test_maintainability_ratchet.pyfailure onmainis unrelated to this diff — it also fails here for that reason alone, and I left the release-ceiling decision to its owner rather than widening a budget.Frontend / Visual Evidence
public_fixture.Type of Change
LoopX Area
Technical Direction
Shared-authority RFC fixture impact
Boundary Checklist
.loopx/,.codex/goals/, and liveACTIVE_GOAL_STATE.md).none.Signed-off-bytrailer (git commit -s).