Skip to content

Handle missing log in export_report#55

Merged
Komzpa merged 1 commit into
konturio:masterfrom
maumaps:codex/fix-keyerror--log--on-geocint
May 14, 2026
Merged

Handle missing log in export_report#55
Komzpa merged 1 commit into
konturio:masterfrom
maumaps:codex/fix-keyerror--log--on-geocint

Conversation

@Komzpa
Copy link
Copy Markdown
Member

@Komzpa Komzpa commented May 14, 2026

Summary

  • keep exported targetLog as an empty string when the performance record has log: None
  • keep the same empty-string behavior when the log key is absent
  • move the branch source from konturio to maumaps

Testing

  • git diff --check origin/master...maumaps/codex/fix-keyerror--log--on-geocint
  • previous exact-head validation on 81bce12: pytest -q, ruff check make_profiler tests/test_report_export.py, pylint --disable=all --enable=E make_profiler, and GitHub CI/Lint passed on Handle missing log in export_report #33

Summary by CodeRabbit

  • Bug Fixes

    • Fixed report export to properly normalize missing or empty log values, ensuring consistent output formatting.
  • Tests

    • Added test coverage to validate the correct handling of missing log values in exported reports.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 14, 2026

Walkthrough

The PR fixes log path normalization in report_export by changing from rec.get("log", "") to rec.get("log") or "", ensuring that present-but-falsy log values (such as None) are consistently exported as empty strings. Test coverage validates both explicit None and missing key scenarios.

Changes

Log path normalization in report export

Layer / File(s) Summary
Log path normalization fix and test coverage
make_profiler/report_export.py, tests/test_report_export.py
export_report computes log_path using or "" to normalize falsy values. Test helper _export_single_report prepares isolated test environment with state reset. Two test functions validate targetLog becomes empty string when record has log=None or omits the log key entirely.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly addresses the main change: normalizing missing or falsy log values in export_report to ensure targetLog is always a string.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/test_report_export.py`:
- Around line 6-17: Add a concise docstring to the helper function
_export_single_report describing its purpose, inputs (tmp_path, monkeypatch,
record) and returned value, then add short inline comments to mark the setup
phase (changing to tmp_path and clearing report_export.status and
report_export.status_list) and the export phase (calling
report_export.export_report and reading report.json) so each logical block is
documented for tooling and readers.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 750492e3-c887-4ae3-8731-cccb4ae37286

📥 Commits

Reviewing files that changed from the base of the PR and between 574bd2f and 81bce12.

📒 Files selected for processing (2)
  • make_profiler/report_export.py
  • tests/test_report_export.py

Comment on lines +6 to +17
def _export_single_report(tmp_path, monkeypatch, record: dict[str, object]) -> dict:
monkeypatch.chdir(tmp_path)
report_export.status.clear()
report_export.status_list.clear()

report_export.export_report(
{"build": record},
{"build": "Build target"},
{"build"},
)

return json.loads((tmp_path / "report.json").read_text())
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Add a helper docstring and brief logical-block comments.

Please document this helper and annotate its setup/export phases to satisfy the Python documentation/comment requirements.

Proposed update
 def _export_single_report(tmp_path, monkeypatch, record: dict[str, object]) -> dict:
+    """Export a single-target report.json and return parsed JSON content."""
+    # Isolate filesystem side effects and reset module-level accumulators.
     monkeypatch.chdir(tmp_path)
     report_export.status.clear()
     report_export.status_list.clear()
 
+    # Export one synthetic target record.
     report_export.export_report(
         {"build": record},
         {"build": "Build target"},
         {"build"},
     )
 
+    # Read the generated report payload.
     return json.loads((tmp_path / "report.json").read_text())
As per coding guidelines, `**/*.py`: "Write docstrings in Python code, they will get used for call graph and generated documentation." and "Write comments for each logical block in Python code."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/test_report_export.py` around lines 6 - 17, Add a concise docstring to
the helper function _export_single_report describing its purpose, inputs
(tmp_path, monkeypatch, record) and returned value, then add short inline
comments to mark the setup phase (changing to tmp_path and clearing
report_export.status and report_export.status_list) and the export phase
(calling report_export.export_report and reading report.json) so each logical
block is documented for tooling and readers.

@Komzpa Komzpa merged commit 340da2b into konturio:master May 14, 2026
7 checks passed
@Komzpa Komzpa deleted the codex/fix-keyerror--log--on-geocint branch May 14, 2026 18:16
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