Skip to content

Fix/preference distillation original date#533

Merged
GenerQAQ merged 2 commits intomemodb-io:devfrom
mbt1909432:fix/preference-distillation-original-date
Apr 1, 2026
Merged

Fix/preference distillation original date#533
GenerQAQ merged 2 commits intomemodb-io:devfrom
mbt1909432:fix/preference-distillation-original-date

Conversation

@mbt1909432
Copy link
Copy Markdown
Contributor

@mbt1909432 mbt1909432 commented Apr 1, 2026

Why We Need This PR?

Problem

When users upload historical chat messages with original_date set in session configs, the skill learner correctly uses the historical date for task distillation, but user preferences distillation was missing the original_date.

This caused preference MQ messages to have original_date: null, leading to skill files being written with today's date instead of the historical session date.

Production logs showed:

{"original_date": null, "task_id": "00000000-0000-0000-0000-000000000000", ...}
  • task_id: "00000000-..." indicates user preferences distillation (not a real task).
  • original_date: null was the bug.

Root Cause

task_agent_curd in task.py has two code paths for publishing SkillLearnDistilled:

  1. Task Success/Fail distillation – had original_date
  2. User Preferences distillation – was missing original_date

Solution

Fetch session.configs.original_date at the start of task_agent_curd and pass it when publishing preferences.

Code Change

File: src/server/core/acontext_core/llm/agent/task.py

async with DB_CLIENT.get_session_context() as db_session:
    # Get session configs to extract original_date
    r_sess = await SD.fetch_session(db_session, session_id)
    session, _ = r_sess.unpack()
    original_date = (
        session.configs.get("original_date")
        if session and session.configs
        else None
    )

    # ... existing task fetch logic ...

# ... later when publishing preferences:
await publish_mq(
    EX.learning_skill,
    RK.learning_skill_agent,
    SkillLearnDistilled(
        ...
        original_date=original_date,  # Added this line
    ).model_dump_json(),
)

Implementation Tasks

  • Add original_date fetching from session.configs in task_agent_curd
  • Merge session fetch into existing DB_CLIENT.get_session_context() block (avoid breaking atomicity tests)
  • Pass original_date when publishing SkillLearnDistilled for user preferences

Files Changed

File Change
src/server/core/acontext_core/llm/agent/task.py Add original_date fetch and pass to preferences MQ

Impact Areas

Which part of Acontext would this feature affect?

  • Client SDK (Python)
  • Client SDK (TypeScript)
  • Core Service
  • API Server
  • Dashboard
  • CLI Tool
  • Documentation
  • Other: ...

User preferences distillation was missing original_date, causing
skill learner to use today's date instead of the historical date.

- Fetch session.configs.original_date at task agent start
- Pass original_date when publishing SkillLearnDistilled for preferences

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@mbt1909432 mbt1909432 requested a review from a team as a code owner April 1, 2026 06:54
@mbt1909432 mbt1909432 force-pushed the fix/preference-distillation-original-date branch 2 times, most recently from c4aac51 to 8bc0276 Compare April 1, 2026 07:09
@GenerQAQ
Copy link
Copy Markdown
Contributor

GenerQAQ commented Apr 1, 2026

CI Failure Analysis + Suggestion

The 3 failing tests in test_task_agent_atomicity.py all report RuntimeError: coroutine raised StopIteration.

Root cause: These tests mock DB_CLIENT.get_session_context() with a side_effect list to return different mock sessions in order. The PR adds a new, separate get_session_context() block for SD.fetch_session, which consumes an extra entry from the list and exhausts it before the test expects.

Suggestion: Merge SD.fetch_session into the existing DB session block (current L132-143) instead of opening a new one. All three functions (fetch_session, fetch_current_tasks, fetch_planning_task) take the same AsyncSession parameter and are read-only queries — safe to share a single session context. This is consistent with how skill_learner.py (L40-54) already does it.

Roughly:

original_date = None
async with DB_CLIENT.get_session_context() as db_session:
    r_sess = await SD.fetch_session(db_session, session_id)
    sess, _ = r_sess.unpack()
    if sess and sess.configs:
        original_date = sess.configs.get("original_date")

    r = await TD.fetch_current_tasks(db_session, session_id)
    tasks, eil = r.unpack()
    if eil:
        return r
    r_plan = await TD.fetch_planning_task(db_session, session_id)
    planning_task, _ = r_plan.unpack()
    ...

This avoids an extra connection pool checkout and preserves the existing tests' expectations on get_session_context call count.

Consolidate SD.fetch_session and TD.fetch_current_tasks into a single
DB_CLIENT.get_session_context() block to avoid exhausting mock side effects
in test_task_agent_atomicity.py tests.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@GenerQAQ GenerQAQ left a comment

Choose a reason for hiding this comment

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

LGTM! Clean fix — correctly wires original_date into the preferences distillation path.

One minor suggestion: consider mirroring the eil-guarded pattern from skill_learner.py (L51-58) for consistency and observability:

session, eil = r_sess.unpack()
original_date = None
if not eil and session and session.configs:
    original_date = session.configs.get("original_date")
    LOG.info("task_agent.original_date_parsed", session_id=str(session_id), original_date=original_date)

Not a blocker — the current code is functionally correct.

@GenerQAQ GenerQAQ merged commit 29e370c into memodb-io:dev Apr 1, 2026
2 checks passed
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