Skip to content

fix(dump): format the dump card time from the epoch milliseconds Dump Row already holds. - #73

Merged
terabytesoftw merged 1 commit into
mainfrom
feat/phase-5-dump-cells
Sep 25, 2026
Merged

terabytesoftw merged 1 commit into
mainfrom
feat/phase-5-dump-cells

Conversation

@terabytesoftw

Copy link
Copy Markdown
Contributor

Pull Request

  • Breaking change (fix or feature that would cause existing functionality to change)
  • Bugfix (non-breaking change that fixes an issue)
  • CI/build configuration
  • Documentation update
  • New feature (non-breaking change that adds functionality)
  • Refactoring (no functional changes)

Related Issues

@terabytesoftw terabytesoftw added the bug Something isn't working label Sep 25, 2026
@coderabbitai

coderabbitai Bot commented Sep 25, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 1d3f9315-f685-4237-a214-e2a5518d74a3

📥 Commits

Reviewing files that changed from the base of the PR and between 1357cc4 and c206fe3.

📒 Files selected for processing (3)
  • src/Panel/Dump/DumpCardRenderer.php
  • tests/Panel/Dump/DumpCardRendererTest.php
  • tests/Storage/SnapshotStoreTest.php

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📜 Recent review details
⏰ Context from checks skipped due to timeout. (20)
  • GitHub Check: phpunit / PHP 8.4-ubuntu-latest
  • GitHub Check: phpunit / PHP 8.4-windows-2022
  • GitHub Check: quality / EditorConfig (ubuntu-latest)
  • GitHub Check: quality / Actionlint (ubuntu-latest)
  • GitHub Check: phpunit / PHP 8.3-ubuntu-latest
  • GitHub Check: quality / Spelling (ubuntu-latest)
  • GitHub Check: composer-require-checker / PHP 8.5-ubuntu-latest
  • GitHub Check: phpunit / PHP 8.5-ubuntu-latest
  • GitHub Check: phpunit / PHP 8.5-windows-2022
  • GitHub Check: phpunit / PHP 8.3-windows-2022
  • GitHub Check: mutation / PHP 8.5-ubuntu-latest
  • GitHub Check: phpstan / PHP 8.5-ubuntu-latest
  • GitHub Check: easy-coding-standard / PHP 8.5-ubuntu-latest
  • GitHub Check: Verify Vite build reproduces dist.
  • GitHub Check: Analyze (actions)
  • GitHub Check: mutation / PHP 8.5-ubuntu-latest
  • GitHub Check: phpunit / PHP 8.4-windows-2022
  • GitHub Check: phpunit / PHP 8.5-windows-2022
  • GitHub Check: phpunit / PHP 8.3-windows-2022
  • GitHub Check: Verify Vite build reproduces dist.
🧰 Additional context used
🪛 PHPMD (2.15.0)
tests/Storage/SnapshotStoreTest.php

[warning] 19-1622: The class SnapshotStoreTest has 1604 lines of code. Current threshold is 1000. Avoid really long classes. (undefined)

(ExcessiveClassLength)


[warning] 19-1622: The class SnapshotStoreTest has 55 public methods and attributes. Consider reducing the number of public items to less than 45. (undefined)

(ExcessivePublicCount)


[warning] 19-1622: The class SnapshotStoreTest has 59 non-getter- and setter-methods. Consider refactoring SnapshotStoreTest to keep number of methods under 25. (undefined)

(TooManyMethods)


[warning] 19-1622: The class SnapshotStoreTest has 55 public methods. Consider refactoring SnapshotStoreTest to keep number of public methods under 10. (undefined)

(TooManyPublicMethods)


[warning] 19-1622: The class SnapshotStoreTest has an overall complexity of 76 which is very high. The configured complexity threshold is 50. (undefined)

(ExcessiveClassComplexity)

src/Panel/Dump/DumpCardRenderer.php

[error] 93-93: Avoid using static access to class '\PHPForge\Debug\Helper\Format' in method 'formatTime'. (undefined)

(StaticAccess)

tests/Panel/Dump/DumpCardRendererTest.php

[warning] 19-750: The class DumpCardRendererTest has 31 non-getter- and setter-methods. Consider refactoring DumpCardRendererTest to keep number of methods under 25. (undefined)

(TooManyMethods)


[warning] 19-750: The class DumpCardRendererTest has 29 public methods. Consider refactoring DumpCardRendererTest to keep number of public methods under 10. (undefined)

(TooManyPublicMethods)


[error] 148-152: Avoid using static access to class '\PHPForge\Debug\Panel\Dump\DumpCardRenderer' in method 'testRenderMessageCellFormatsMillisecondsAtTheUpperBoundary'. (undefined)

(StaticAccess)


[error] 213-217: Avoid using static access to class '\PHPForge\Debug\Panel\Dump\DumpCardRenderer' in method 'testRenderMessageCellKeepsTimeAndTraceMetadataTogether'. (undefined)

(StaticAccess)


[error] 420-424: Avoid using static access to class '\PHPForge\Debug\Panel\Dump\DumpCardRenderer' in method 'testRenderMessageCellRendersFormattedTimeWhenTimeIsPositive'. (undefined)

(StaticAccess)

🔇 Additional comments (3)
tests/Storage/SnapshotStoreTest.php (1)

8-8: LGTM!

Also applies to: 488-488, 902-902, 1510-1510, 1533-1533

src/Panel/Dump/DumpCardRenderer.php (1)

83-85: LGTM!

Also applies to: 93-93

tests/Panel/Dump/DumpCardRendererTest.php (1)

149-149: LGTM!

Also applies to: 165-165, 214-214, 421-421


📝 Summary

Summary by CodeRabbit

  • Bug Fixes
    • Corrected dump card time display for timestamps stored in milliseconds. Fractional milliseconds are truncated, and timestamps at or below zero continue to display as blank. This provides consistent time-of-day values for dump entries without changing how empty or non-positive timestamps appear.

Walkthrough

Dump timestamps are formatted as epoch milliseconds. Permission-dependent snapshot storage tests are marked as Linux-only with a PHPUnit attribute.

Changes

Dump timestamp formatting

Layer / File(s) Summary
Millisecond timestamp formatting
src/Panel/Dump/DumpCardRenderer.php, tests/Panel/Dump/DumpCardRendererTest.php
formatTime() passes the timestamp value to Format::timeOfDay() without multiplying by 1,000. Test fixtures use millisecond-scale values and describe fractional millisecond truncation.

Snapshot storage test platform annotations

Layer / File(s) Summary
Linux-only storage tests
tests/Storage/SnapshotStoreTest.php
Four permission-dependent tests use RequiresOperatingSystemFamily instead of runtime Windows skip branches.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to c206f

Dump timestamps now use the row’s documented millisecond units, and permission-sensitive storage tests are limited to Linux. The supplied evidence establishes no concrete user-impacting or workflow-blocking regression.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 3 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description check ✅ Passed The description identifies the pull request as a non-breaking bugfix, which matches the timestamp correction and test updates.
Title check ✅ Passed The title clearly identifies the main change: formatting dump card time from the epoch milliseconds stored by DumpRow.
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.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

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

A rabbit checks the clock’s new scale,
Milliseconds now mark the trail.
Linux tests declare their range,
No skip branch remains to change.
I nibble greens and hop away,
With tidy tests to greet the day.

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

@codecov

codecov Bot commented Sep 25, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (1357cc4) to head (c206fe3).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@             Coverage Diff             @@
##                main       #73   +/-   ##
===========================================
  Coverage     100.00%   100.00%           
  Complexity      2391      2391           
===========================================
  Files            173       173           
  Lines           8723      8723           
===========================================
  Hits            8723      8723           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@terabytesoftw
terabytesoftw merged commit 2a84310 into main Sep 25, 2026
43 checks passed
@terabytesoftw
terabytesoftw deleted the feat/phase-5-dump-cells branch September 25, 2026 21:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant