Repository navigation
fix(logging): create the log file on the first record - #5167
Merged
Merged
Conversation
paul-nechifor
requested review from
Dreamsorcerer,
TomCC7,
aclauer,
jeff-hykin,
leshy,
mustafab0 and
spomichter
as code owners
November 10, 2026 08:19
Contributor
 [Medium impact] The PR changes file creation and routing across DimOS logging, including run startup. The handler cleanup requirement must be satisfied before merging. Findings
SummaryThis PR opens log files only when the first record is written and moves early records into the run's
Reviews (1) · Last reviewed commit: "fix(logging): create the log file on the..." · Reviewed by Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
go2 replay realtime (arm64)
Details
| Benchmark suite | Current: 60c2c4d | Previous: 1916f35 | Ratio |
|---|---|---|---|
peak memory |
1733.07 MB |
1740.68 MB |
1.00 |
peak threads |
374 threads |
377 threads |
0.99 |
network (transport) |
2733.739 MB |
2730.983 MB |
1.00 |
disk write |
1.852 MB |
1.586 MB |
1.17 |
instructions |
137.219 G |
138.613 G |
0.99 |
This comment was automatically generated by workflow using github-action-benchmark.
Codecov Report❌ Patch coverage is
@@ Coverage Diff @@
## main #5167 +/- ##
=======================================
Coverage 83.74% 83.75%
=======================================
Files 1866 1866
Lines 182158 182200 +42
Branches 15008 15012 +4
=======================================
+ Hits 152549 152594 +45
+ Misses 25569 25565 -4
- Partials 4040 4041 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 3 files with indirect coverage changes 🚀 New features to boost your workflow:
|
github-actions
Bot
added
the
ready-to-merge
paul-nechifor
enabled auto-merge (squash)
November 11, 2026 04:07
github-actions
Bot
removed
the
ready-to-merge
TomCC7
approved these changes
Nov 11, 2026
Every process that imported a DimOS logger opened logs/dimos_<timestamp>_<pid>.jsonl at import time, so short CLI commands left an empty file behind. The file handler now opens the file lazily. dimos run also logged a few records before the run directory existed. Those records now move into the run's main.jsonl.
test_forked_child_cannot_move_parent_startup_log fails on every run with --cov. The CI re-run step uses --no-cov, so the failure looked flaky. conftest starts coverage in every test subprocess. The test's script runs in tmp_path, where "dimos" in source is a package name and not a directory. Coverage starts a second instance in the forked child, and that instance counts the package as measured only when dimos/__init__.py runs. A fork never imports it again, so the child prints the warning on stderr, which the test requires to be empty.
paul-nechifor
force-pushed
the
paul/fix/no-empty-cli-log-files
branch
from
November 11, 2026 10:29
8021e3d to
60c2c4d
Compare
github-actions
Bot
added
the
ready-to-merge
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why do we need this?
Every
dimosCLI call left alogs/dimos_<timestamp>_<pid>.jsonlfile behind, even when the command logged nothing. Commands likedimos statusordimos topic echoeach added an empty file. One test checkout had about 55,000 of these files, which makesls logsuseless.dimos runhad a smaller version of the same problem. It logs "Starting DimOS" before the run directory exists, so each run left a one-line file next to itslogs/<run-id>/directory.What is this feature?
The log file handler opened its file as soon as a logger was created, which happens at import time. It now opens the file when the first record is written. A process that logs nothing leaves no file.
When
dimos runswitches to the per-run directory, the records it already wrote are moved into the run'smain.jsonland the per-process file is removed.dimos lognow shows those early records too.Both places that built the file handler now share one helper.