Skip to content

fix(stats): preserve history when appending records fails - #1039

Draft
u7-u7 wants to merge 1 commit into
modelscope:mainfrom
u7-u7:fix/atomic-workflow-stats-20261030
Draft

u7-u7 wants to merge 1 commit into
modelscope:mainfrom
u7-u7:fix/atomic-workflow-stats-20261030

Conversation

@u7-u7

@u7-u7 u7-u7 commented Oct 30, 2026

Copy link
Copy Markdown

Summary

Preserve previously recorded workflow token/timing statistics if appending a new record fails partway through a file write.

append_stats currently opens the cumulative history with w before dumping JSON. A short write (for example ENOSPC) truncates the previous history and leaves invalid JSON. The agent tool logs the write failure; the next append then resets the unreadable history to an empty list. A serialization failure can damage the same history too.

Use the existing atomic_write_json helper under the existing per-path asyncio lock. Preserve ensure_ascii=True and indent=2; a temporary file is replaced only after a complete write. No new dependencies or schema changes.

Validation

  • Exact main base 6d90d84e55d508fcb52ea19f3912de6f6a477e38: serialization and injected one-byte-then-ENOSPC regressions damage the old file as expected. The new replace-failure test also fails on baseline because the old implementation does not use replacement.
  • python -m pytest tests/utils/test_stats.py tests/utils/test_atomic_file.py -q: 9 passed.
  • python -m pytest tests/utils -q: 151 passed, 30 subtests passed, 1 failed. The failure is the existing transcript-path expectation in TestAgentToolTranscript::test_save_transcript_writes_message_objects_and_dicts; independently reproduced with unchanged stats.py. It expects <output>/subagents/... rather than the implementation's .ms_agent/subagents/....
  • Changed-file pre-commit hooks and git diff --check: passed.

Tests use real temporary files and call production append_stats; the ENOSPC fault wraps both legacy open and atomic-helper fdopen writers, writes one actual byte and then raises. They verify the old file remains byte-for-byte unchanged, temporary files are removed, and the next successful append preserves previous records.

Limits

No live agent/model calls or full project suite. Existing datetime.utcnow deprecation warnings remain. This does not add multi-process locking or fsync/power-loss durability guarantees; it retains the helper's existing behavior. Existing unreadable-history reset behavior is unchanged.

AI-assisted implementation and automated review; draft for maintainer review, without a claim of human review.

Checklist

  • Focused regression tests added and passing
  • Changed-file pre-commit checks passed using existing cached environments
  • Full-repository hooks pass (not claimed; a previous run on this same upstream base reported unrelated formatting failures)

The final local commit did not rerun hooks: the default cache was read-only, so it used the already-passing explicit cached changed-file check. An initial mistyped cache path began environment initialization and was stopped; it is not a passing check. No new dependency or workflow configuration is included.

This branch has not been deployed

No deployments
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