Skip to content

[FIX]: Preserve reports with duplicate timestamps - #179

Merged
Nina Chikanov (nina-msft) merged 3 commits into
microsoft:mainfrom
rioyu123:codex/fix-report-filename-collisions
Sep 4, 2026
Merged

[FIX]: Preserve reports with duplicate timestamps#179
Nina Chikanov (nina-msft) merged 3 commits into
microsoft:mainfrom
rioyu123:codex/fix-report-filename-collisions

Conversation

@rioyu123

@rioyu123 Rio Yu (rioyu123) commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Description

JsonFileReportSink used a seconds-precision timestamp with write_text(). Two reports emitted within the same second therefore resolved to the same path, allowing the later report to silently replace the first.

This change preserves every report while keeping the common filename concise:

  • Report filenames now use a UTC timestamp with millisecond precision, for example run_report_2026-04-25T14-30-00-123.json.
  • The timestamp path is created exclusively with mode x, so an existing report is never overwritten.
  • A UUID suffix is added only when the timestamp path already exists; the fallback path is also created exclusively.
  • Reports continue to be fully serialized before any output file is opened, so serialization failures do not leave empty report files.
  • Report content is written with explicit UTF-8 encoding instead of the platform default.

The usage guide and API documentation now describe the concise common filename, collision fallback, and remaining error cases. Tests cover repeated timestamps, existing-file preservation, a forced fallback collision, and serialization failure.

Validation:

  • uv run pre-commit run --all-files — passed
  • uv run pytest tests/unit/reporting -q — 46 passed
  • uv run pytest tests/unit -q — 1035 passed on Linux; 1033 passed and 2 skipped on Windows
  • uv run --group docs mkdocs build --strict — passed on Linux

Breaking changes

There are no public API or report-schema changes. Generated filenames now include milliseconds and may include a UUID suffix on collision. Consumers using run_report_*.json are unaffected; consumers parsing the exact timestamp format should account for the additional millisecond segment.

Checklist

  • pre-commit run --all-files passes
  • Tests added or updated for changes
  • Documentation updated

@rioyu123
Rio Yu (rioyu123) requested a review from a team August 27, 2026 09:15
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

@rioyu123

Copy link
Copy Markdown
Contributor Author

@microsoft-github-policy-service agree

@rioyu123
Rio Yu (rioyu123) force-pushed the codex/fix-report-filename-collisions branch from 7a7f44e to 489675a Compare August 27, 2026 12:59

@nina-msft Nina Chikanov (nina-msft) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for flagging this Rio Yu (@rioyu123)! Let me know what you think of the following comments :-)

Comment thread rampart/reporting/json_file.py Outdated
Comment thread rampart/reporting/json_file.py Outdated
Comment thread rampart/reporting/json_file.py Outdated
@nina-msft
Nina Chikanov (nina-msft) dismissed their stale review August 28, 2026 00:49

Whoops - meant to comment only instead of approval :-)

Signed-off-by: Rio Yu <52408936+rioyu123@users.noreply.github.com>
Comment thread rampart/reporting/json_file.py Outdated
Comment thread docs/usage/results-and-reporting.md Outdated

@nina-msft Nina Chikanov (nina-msft) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for your contribution Rio Yu (@rioyu123) :-)

@nina-msft
Nina Chikanov (nina-msft) merged commit 7395a37 into microsoft:main Sep 4, 2026
17 of 18 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