fix(automl): safely append training logs

[林SO (Linxiushen)]

Why are these changes needed?

Appending to an AutoML training log restarts record_id at zero, so an appended run's checkpoint can reconstruct an older configuration. Continue IDs after the greatest existing record ID, skip checkpoint entries during that scan, and keep each fit's own best-record tracking. Existing duplicate IDs are not rewritten. A valid final record or checkpoint without a newline receives a separator before appending.

The latest follow-up removes the whole-search writer lock and the added filelock dependency, as requested in the maintainer review. Logging no longer makes independent fits wait for another fit's writer to close. Concurrent independent fits sharing a filename remain unsupported: records can mix or corrupt, IDs can repeat, and normal writes can overwrite another log. Use a distinct log_file_name for each concurrent fit. The documentation explains per-call filenames, sequential append versus overwrite, and that appending logs does not resume a previous search.

Validation

• Before the latest follow-up, both new nonblocking-writer regressions failed on the current maintainer-merged head 6e669d4412e73addc28eb7c699ee3f14d8f3498e. A separate bounded exact-module probe reproduced the wait in append and overwrite modes.
• Windows / Python 3.10.20: all 24 focused parametrized cases pass through pytest using a local plain temporary-directory fixture. The host's normal pytest numbered-directory fixture encounters existing Windows path/cleanup permission problems; a normal focused run timed out after 150 seconds. That failed run is retained separately and is not reported as passed.
• The same 24 test functions also pass through a separate direct temporary-path harness. Cases cover record reconstruction, sparse and duplicate legacy IDs, checkpoint-only logs, current-fit best records, LF/CRLF/CR/unterminated tails, nonblocking writer opens, error cleanup, and overwrite behavior.
• An offline public-API smoke runs four RF AutoML.fit calls on one instance. Sequential append, independently supplied filenames, predictions, estimator reconstruction, and default overwrite pass; record counts are [2, 4, 2, 2].
• All applicable configured pre-commit hooks pass for the four changed files; hooks with no matching files are skipped.

Shared-file concurrent write correctness is not promised or tested. No full AutoML, forecasting, Spark, GPU, Windows ARM, or cloud suite was run. Hosted CI and maintainer approval remain separate gates.

[Li Jiang (thinkall)]

Overall review of the complete PR: changes are required.

1. flaml/automl/training_log.py:74: appending after a valid final JSON record without a trailing newline concatenates records as }{ and corrupts all subsequent log readers. Detect an unterminated nonempty tail and add a newline before writing; test record and checkpoint tails.
2. flaml/automl/training_log.py:68-74: concurrent appenders scan and allocate IDs without a lock, so both can choose the same next ID. Hold a cross-process lock from ID discovery through writer close and add a two-writer uniqueness regression.

Posted by thinkall-agent-auto-reviewer

[林SO (Linxiushen)]

Addressed both findings in the follow-up commit:

1. Appending now adds a separator only when a nonempty log ends with neither LF nor CR. The tests cover record and checkpoint tails with no newline, LF, CRLF, and CR, including Windows text-mode behavior.
2. Both normal and append writers now hold a filelock sidecar lock from before opening/scanning the log until flush and close. Real spawned-process tests cover missing and existing shared logs, a normal writer followed by an appender, and independent files. Error-path tests verify release after malformed logs, failed opens, and context exceptions. The docs explain the serialization and its effect on time_budget.

The original follow-up regressions fail on Linux with corrupt JSON and duplicate IDs ([0, 0] / [0, 1, 1]). The final 25-case suite passes on Linux Python 3.10, 3.11, 3.12, and 3.13; all pre-commit hooks pass. Windows Python 3.10/3.12 each pass the 21 nonprocess cases, while local CreateProcess access denial prevents Windows multiprocessing validation. Core-only imports were also checked without the new optional dependency installed.

The PR description now reflects the locking behavior, expanded validation, and remaining validation limits. Ready for another review.

[Li Jiang (thinkall)]

Overall review of the complete current PR: approved.

The writer lock now covers ID discovery through file close, preventing duplicate IDs across concurrent writers. Appends safely insert separators after valid unterminated record or checkpoint tails. Regression and stress coverage exercises missing/existing logs, legacy sparse IDs, LF/CRLF/CR/no terminator, threads/processes, malformed files, failed opens, crash recovery, and retrain/log readers. No blocking issues found.

Posted by thinkall-agent-auto-reviewer

[Copilot]

🟢 Approval recommended

No blocking issues were identified in the append handling, lock lifecycle, dependencies, or regression coverage.

No open issues 🎉

What changed in this PR

Makes AutoML training-log appends safe across sequential and concurrent writers.

Changes:

• Resumes record IDs after the existing maximum and handles missing trailing newlines.
• Adds cross-process locking and the filelock dependency.
• Adds regression coverage and documents shared-log waiting behavior.

File	Description
test/​automl/​test_training_log_append.py	Tests IDs, reconstruction, line endings, concurrency, and lock cleanup.
pyproject.toml	Adds filelock to AutoML and test extras.
flaml/​automl/​training_log.py	Implements safe appending and writer locking.
flaml/​automl/​automl.py	Documents serialization and time-budget implications.

_{Review effort: Balanced}

[Copilot]

🔵 Needs a closer look

Windows cross-process locking and successful end-to-end AutoML.fit integration remain unvalidated.

No open issues 🎉

_{Review effort: Balanced}

[Copilot]

🔵 Needs a closer look

Writer locking affects every logged fit, while successful fit integration and Windows multiprocessing remain unvalidated.

No open issues 🎉

_{Review effort: Balanced}

[Li Jiang (thinkall)]

Thanks for addressing the duplicate record IDs when appending training logs. I’m concerned that the whole-search file lock introduces a broader behavioral change than the original bug requires.

The lock is acquired before  _search()  and held until it finishes, with no acquisition timeout. Separate AutoML fits sharing a filename can therefore serialize their searches—or wait indefinitely behind a stuck fit. Although waiting counts toward  time_budget , that budget does not bound the lock wait. This also affects default  append_log=False , where the waiting writer eventually overwrites the earlier log anyway. Logging should not become a blocker for training. Parallel trials within one fit already use a single coordinator-owned training-log writer, so they do not require this cross-fit serialization.

I suggest keeping the fix lightweight: continue record IDs after the greatest existing ID when opening for append, skip checkpoint entries during that scan, preserve each fit’s own best-record tracking, and handle an existing final line without a newline. Please remove the whole-search locking and its associated dependency from this change.

The documentation should explicitly clarify:

• Concurrent independent fits writing to the same file can mix or corrupt logs, produce duplicate record IDs, or overwrite each other’s logs. Use a distinct log file for each fit to avoid these conflicts.
•  log_file_name  can be supplied separately to each  fit()  call, including repeated calls on one AutoML instance.
• Sequential reuse with  append_log=True  preserves records with continuing IDs; default  append_log=False  overwrites the file.
• Appending logs does not resume the previous search.

This would address the original correctness bug without introducing unbounded training waits.

[Copilot]

🔵 Needs a closer look

Windows multiprocessing remains unverified, and no successful end-to-end AutoML fit smoke is available to validate the locking integration.

No open issues 🎉

_{Review effort: Balanced}

[林SO (Linxiushen)]

Removed the whole-search lock and the added filelock dependency. Sequential appends still continue after the maximum record ID, skip checkpoints while scanning, preserve the current fit's best record, and separate unterminated final lines.

The docs now cover distinct filenames for concurrent fits, per-call log_file_name on a reused AutoML instance, append versus overwrite, and that appending does not resume a search. The two new nonblocking-writer regressions fail before this change. Afterward, all 24 focused cases pass through pytest with a plain temporary-directory fixture; the host's normal numbered-directory fixture had a separate Windows path/cleanup failure. Four sequential RF fits on the same instance also pass, covering append, per-call filenames, and overwrite. Applicable pre-commit hooks pass. Shared-file concurrent write safety and a complete AutoML suite are not claimed.

[Copilot]

🟢 Approval recommended

The focused implementation and regression coverage address sequential append correctness with no unresolved findings.

No open issues 🎉

_{Review effort: Balanced}

[Li Jiang (thinkall)]

Reviewed the new nonblocking logging follow-up at 9c1b0e6bfb954ac9a6fe256306342d5dc63e6718 with review-code.

The lock-state and dependency removal leaves sequential append IDs, checkpoint selection/reload, newline-tail handling, and cleanup intact. Both public append_log descriptions now explicitly disclose that independent fits sharing a log can corrupt or overwrite records, require distinct per-fit filenames, and distinguish appending from search resumption. No safe simultaneous-write guarantee is inferred. No issue found in this new delta.

Validation: python -m pytest test\automl\test_training_log_append.py -q --disable-warnings passed (24 passed), including the two-second nonblocking assertion in both open modes, sequential/checkpoint cases, overwrite behavior, and error/pickle cleanup. No FileLock/filelock Python references remain; the two optional dependency removals match the source. The Low harness was CLEAN and its exact-head provenance was audited and committed locally.

This is not an approval: current-head CI is still incomplete or failing. Approval remains subject to successful actual workflow runs and all same-head discussion/Copilot/code-review gates.

Posted by thinkall-agent-auto-reviewer

[Li Jiang (thinkall)]

Overall re-review of the complete updated PR at 9c1b0e6bfb954ac9a6fe256306342d5dc63e6718: approved.

I rechecked the complete diff against current main and reused the audited CLEAN review-code evidence for this unchanged source SHA. Sequential appends choose the next unused record ID, preserve existing records and valid newline/checkpoint tails, and keep checkpoint reloads addressable. Missing/invalid-file handling and cleanup remain intact. The nonblocking behavior is explicitly documented: independent concurrent fits must use distinct log filenames; no shared-file serialization guarantee is implied.

The exact-head validation previously passed python -m pytest test\automl\test_training_log_append.py -q --disable-warnings (24 passed); source has not changed since that run. All current-head discussion threads are resolved, Balanced Copilot explicitly recommends approval, applicable check contexts pass, and the actual Build, docs, and Code formatting workflow runs have now completed successfully. No remaining issue found.

Posted by thinkall-agent-auto-reviewer