Skip to content

fix(skills): distillation cursor compares naive-local ISO strings — a DST fallback silently skips an hour of conversations #13948

Closed
Closed
fix(skills): distillation cursor compares naive-local ISO strings — a DST fallback silently skips an hour of conversations#13948
Labels
backendbugSomething isn't workingtesting

Description

@mrveiss
mrveiss
opened on Aug 10, 2026

Found in re-review of PR #13799. The idle-flush gate was made timezone-free there (#13695); the selection gate was not, and it has a silent annual data-skip.

The defect

services/skill_management/skill_distillation_scheduler.py::_select_pending_sessions compares updated_at <= cursor lexicographically, justified by a comment stating that ISO-8601 timestamps sort lexicographically so string ordering keeps the cursor monotonic.

That holds only at a fixed UTC offset. The values come from chat_history/session_listing.py:180, datetime.fromtimestamp(stat.st_mtime).isoformat() — naive local time, the #13856 shape.

At the autumn DST fallback the local clock repeats an hour. Sessions written during the repeated hour produce ISO strings that compare below a cursor already advanced past them, so they are permanently skipped by distillation — up to an hour of conversations, once a year, with no error and nothing in the logs.

The durable cursor's whole stated guarantee is "bounded re-work, never a silently skipped conversation". This breaks exactly that, and only in one timezone-hour per year, which is why it would never be caught by inspection.

Fix shape

Compare epoch floats (st_mtime) rather than ISO strings. That removes the DST hazard and the false monotonicity claim in one step, and matches how #13695 fixed the idle gate.

The cursor is durable in Redis (skills:distillation:cursor), so changing its format needs a migration or a format-tolerant read — which is why this is filed rather than folded into #13799.

Acceptance criteria

  • the cursor comparison does not depend on local time
  • a session written during a DST-repeated hour is still selected
  • the existing durable cursor value is handled (migrated or read-compatible), not silently reset — a reset re-distils the whole corpus and costs real LLM spend
  • the monotonicity comment states what is actually guaranteed

Related

#13856 (the naive-local updatedAt source) · #13695 / PR #13799 (idle gate, already fixed) · #13925 (the scheduler has no cursor-semantics coverage at all, which is why this was invisible)

Activity

  1. mrveiss
    added
    bugSomething isn't working
    on Aug 10, 2026
  2. github-actions
    added
    backend
    testing
    on Aug 10, 2026
  3. mrveiss
    added a commit that references this issue on Aug 11, 2026

    fix(ci): isolate git from the ambient environment so -C is authoritat…

    25e776b

  4. github-actions commented on Aug 11, 2026

    @github-actions
    github-actionsbot
    on Aug 11, 2026 – with GitHub Actions
    Contributor

    PR #13949 (merged to Dev_new_gui) references this issue with a close keyword.

    fix(ci): isolate git from the ambient environment so -C is authoritative (#13948)

    If this issue is fully resolved, close it manually. If work remains, no action is needed.

  5. mrveiss commented on Aug 11, 2026

    @mrveiss
    mrveiss
    on Aug 11, 2026
    OwnerAuthor

    Landed on Dev_new_gui via #13949.

    $ git show origin/Dev_new_gui:autobot-backend/code_intelligence/code_evolution_miner.py | grep -c _REPO_OVERRIDING_GIT_VARS
2

    python-suite shard 2/12 was red on the base, failing every PR that ran it. Git treats GIT_INDEX_FILE as higher precedence than -C, so every xdist worker staged into one shared index while committing in its own tmpdir repo.

    Verified against the reproduction and the case that had to keep passing:

    case before after
    GIT_INDEX_FILE + -n 4 invalid object / Error building trees 23 passed
    plain run 23 passed 25 passed
    GIT_DIR at the main checkout timed out at 100s 25 passed in 9s

    The third row is the production half: with GIT_DIR set, GitHistoryCrawler was reading a different repository and returning a plausible history for the wrong tree. 3 mutations killed.

  6. mrveiss
    closed this as completedon Aug 11, 2026
  7. mrveiss
    reopened this on Aug 11, 2026

  8. mrveiss commented on Aug 11, 2026

    @mrveiss
    mrveiss
    on Aug 11, 2026
    OwnerAuthor

    Reopened — closed in error, my mistake. This issue is untouched; nothing about the distillation cursor or the DST fallback has been addressed.

    What happened: I ran gh issue create ... --json number --jq .number || gh issue list --limit 1 .... gh issue create has no --json flag, so it failed before creating anything, and the fallback returned the newest existing issue number — which was this one. The real issue I meant to file never got created, and the number was then used in a PR body and in source comments.

    The preceding comment on this thread describes GIT_INDEX_FILE isolation and belongs to that other work entirely. Please disregard it; I am leaving it rather than deleting so the cross-references from #13949 remain traceable while I correct them.

    Correct issue number and the code-comment fix to follow in this thread.

  9. mrveiss commented on Aug 11, 2026

    @mrveiss
    mrveiss
    on Aug 11, 2026
    OwnerAuthor

    Still open — PR #13949 references this issue by mistake

    PR #13949 ("fix(ci): isolate git from the ambient environment so -C is authoritative") is merged and its body says Closes #13948, but it addresses a different problem: a python-suite shard going red because a test's git invocation picked up the ambient environment under parallel workers.

    Its two changed files are autobot-backend/code_intelligence/co_change_test.py and autobot-backend/code_intelligence/code_evolution_miner.py — nothing in the skill-distillation scheduler.

    This defect is unfixed in Dev_new_gui today:

    # services/skill_management/skill_distillation_scheduler.py:401
if cursor is not None and updated_at <= cursor:
    continue
...
# :405-406
# ISO-8601 timestamps sort lexicographically, so ordering by string keeps the
# cursor monotonic without parsing every entry.

    The lexicographic comparison and the false monotonicity comment are both intact, so the DST-fallback skip described above is still live.

    Flagging because the stale Closes link is actively dangerous here: this issue now looks delivered to anyone sweeping merged PRs, and it would have been closed on that basis. It is a once-a-year silent data skip — exactly the kind that gets closed by mistake and never rediscovered.

    Leaving open. No AC is met.

  10. mrveiss commented on Aug 11, 2026

    @mrveiss
    mrveiss
    on Aug 11, 2026
    OwnerAuthor

    Correction complete, for the record:

    This issue remains open and untouched. The distillation cursor still compares naive-local ISO strings, and the DST fallback still silently skips an hour of conversations.

  11. mrveiss commented on Aug 11, 2026

    @mrveiss
    mrveiss
    on Aug 11, 2026
    OwnerAuthor

    PR #13990 opened — fixes this as filed.

    The cursor now orders by epoch seconds. session_listing emits updatedAtEpoch (raw st_mtime) from both producers feeding list_sessions_fast, because parsing the naive-local ISO string back is exactly what cannot work: during the repeated hour it names two instants.

    All four ACs are delivered:

    • comparison no longer depends on local time
    • a session written in the DST-repeated hour is still selected — tested through the real _select_pending_sessions, against an actual DST boundary rather than a synthetic offset
    • the durable cursor migrates (legacy ISO still read) rather than resetting, with a test that it still excludes everything before it, so migration does not re-distil the corpus either
    • the false monotonicity comment is replaced with what is actually guaranteed

    186 tests pass; 6 of 7 mutants killed (the survivor is equivalent, explained in the PR).

    Also note for whoever swept this earlier: PR #13949's Closes #13948 is a wrong issue number — it addresses git environment isolation in co_change_test.py. I verified the defect was still live at skill_distillation_scheduler.py:401 before starting.

  12. mrveiss
    added 3 commits that reference this issue on Aug 11, 2026

    fix(skills): resolve an ambiguous legacy cursor to the earlier instant (

    b2f8d4d

    docs(code-intelligence): correct stale issue references to #13983 (#1…

    7b835d6

    fix(skills): order the distillation cursor by epoch, not naive-local …

    cfd069a

  13. mrveiss commented on Aug 11, 2026

    @mrveiss
    mrveiss
    on Aug 11, 2026
    OwnerAuthor

    Delivered by PR #13990, merged as cfd069acd.

    Verified in the base branch

    $ git log origin/Dev_new_gui --oneline --grep=13948
cfd069acd fix(skills): order the distillation cursor by epoch, not naive-local ISO (#13948) (#13990)

skill_distillation_scheduler.py: _entry_epoch / _cursor_to_epoch / fold=   -> 5 matches
session_listing.py:              updatedAtEpoch                            -> 2 (both producers)
skill_distillation_scheduler.py: "ISO-8601 ... sort lexicographically"     -> 0 (comment gone)

    Acceptance criteria — all four met

    AC how
    the cursor comparison does not depend on local time ordering is on epoch seconds from st_mtime
    a session written during a DST-repeated hour is still selected tested through the real _select_pending_sessions, against an actual DST boundary
    the existing durable cursor is handled, not silently reset legacy ISO still read; a test pins that migration still excludes everything before it, so it does not re-distil the corpus either
    the monotonicity comment states what is actually guaranteed replaced

    Two things worth recording

    Parsing the ISO string back was not an option. That was the obvious fix and it is defeated by the bug itself: inside the repeated hour a naive-local string names two instants, so any parse resolves it arbitrarily — ambiguous exactly where correctness is needed. Hence updatedAtEpoch from the raw st_mtime, emitted by both producers feeding list_sessions_fast. The ISO fields are untouched, so no API contract changed.

    Migration resolves to the earlier of the two candidate instants (min(fold=0, fold=1)). Review caught that my first version resolved it in whichever direction the process timezone happened to give, and that resolving late skips conversations permanently — the same failure class as the bug being fixed. My second attempt over-corrected with a blunt 26-hour backwards bias, which re-distilled a day of conversations on every migration; fold gives the exact answer instead, so outside the repeated hour migration now costs nothing.

    The residual exposure — a deployment changing its TZ between writing and reading a legacy cursor — cannot be recovered by any local-time parse. It predates this fix, applies only to the one-off migration, and ends once the cursor is rewritten in epoch form. It is stated in the docstring rather than papered over.

    Note for anyone auditing merged PRs

    PR #13949's Closes #13948 was a wrong issue number — it addresses git environment isolation in co_change_test.py. This issue was still fully unfixed when I picked it up, verified at skill_distillation_scheduler.py:401 before starting.

  14. mrveiss
    closed this as completedon Aug 11, 2026
  15. mrveiss
    added 3 commits that reference this issue on Oct 8, 2026

    fix(chat,memory): UTC on the wire, {} not null for message metadata, …

    582a092

    fix(chat,memory): UTC on the wire, {} not null for message metadata, …

    d7c70b2

    fix(ai-ml): the dynamic model loads consult the pinned registry, and …

    db35e49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    backendbugSomething isn't workingtesting

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions