[fix] kimi_cli failed to find CHANGELOG.md#1277
[fix] kimi_cli failed to find CHANGELOG.md#1277ClSlaid wants to merge 3 commits intoMoonshotAI:mainfrom
Conversation
Pyinstaller packed binary will extract itself and then run. However, it does not extract the CHANGELOG.md at the first place, thus lead to instant failure on starting the program. This patch makes reading CHANGELOG.md file lazy, then the file will only be read once when running instead of starting. Signed-off-by: cl <cailue@apache.org>
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Signed-off-by: cl <cailue@apache.org>
There was a problem hiding this comment.
Pull request overview
This PR fixes an issue where PyInstaller-packed binaries fail at startup because CHANGELOG.md is not immediately available when the module is imported. The solution converts the eager loading of CHANGELOG.md (at module import time) to lazy loading (on first access), preventing the startup failure.
Changes:
- Replaced module-level
CHANGELOGconstant with a lazy-loadingget_changelog()function that caches the parsed changelog on first access - Updated the
/changelogcommand to callget_changelog()instead of using theCHANGELOGconstant - Added graceful FileNotFoundError handling that returns an empty changelog if the file is missing
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/kimi_cli/utils/changelog.py | Introduced lazy loading for CHANGELOG.md with caching and error handling; minor formatting change to ReleaseEntry construction |
| src/kimi_cli/ui/shell/slash.py | Updated import and usage from CHANGELOG constant to get_changelog() function |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| result[current_ver] = ReleaseEntry( | ||
| description=description, entries=norm_entries) |
There was a problem hiding this comment.
This formatting change appears to be unintentional. The original code had the entire ReleaseEntry construction on a single line. While the reformatted version is valid, it's inconsistent with the surrounding code style and doesn't add value to the PR's purpose of fixing the PyInstaller issue. Consider reverting this to maintain consistency with the original formatting.
| result[current_ver] = ReleaseEntry( | |
| description=description, entries=norm_entries) | |
| result[current_ver] = ReleaseEntry(description=description, entries=norm_entries) |
| def get_changelog() -> dict[str, ReleaseEntry]: | ||
| """Lazily load and parse the changelog on first access.""" | ||
| global _cached_changelog | ||
| if _cached_changelog is None: | ||
| path = Path(__file__).parent.parent / "CHANGELOG.md" | ||
| try: | ||
| _cached_changelog = parse_changelog( | ||
| path.read_text(encoding="utf-8")) | ||
| except FileNotFoundError: | ||
| _cached_changelog = {} | ||
| return _cached_changelog |
There was a problem hiding this comment.
The new lazy loading mechanism lacks test coverage. While the existing test for parse_changelog will continue to work, there's no test that verifies the lazy loading behavior, the caching mechanism, or the FileNotFoundError handling. Consider adding a test that verifies: 1) the changelog is only loaded once (caching works), 2) calling get_changelog multiple times returns the same cached result, and 3) graceful handling when CHANGELOG.md is missing.
Pyinstaller packed binary will extract itself
and then run. However, it does not extract the
CHANGELOG.md at the first place, thus lead to
instant failure on starting the program.
This patch makes reading CHANGELOG.md file lazy,
then the file will only be read once when running
instead of starting.
Related Issue
Discribed in PR.
Checklist
make gen-changelogto update the changelog.make gen-docsto update the user documentation.