fix(action): align auto mode reporting and base comparison#2807
fix(action): align auto mode reporting and base comparison#2807
Conversation
Use auto-mode specific summary and honor PR comments/annotations in auto mode, matching explicit mode behavior. Track output paths in file results and compare minified sizes against base output artifacts when available to avoid source-vs-output mismatches. Also scope esbuild/yui type enforcement to explicit mode and add tests covering the updated behavior.
|
📝 WalkthroughWalkthroughAdds repository-relative Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Workflow Client
participant Action as Minify Action
participant GitHub as GitHub API
participant Base as Base Branch
Client->>Action: Trigger runAutoMode (produce outputs)
Action->>Action: runMinification -> FileResult(s) with `outputFile`
Action->>Action: normalizeComparePath for each file
alt valid comparePath
Action->>GitHub: fetch file content at comparePath
GitHub->>Base: request file@base
Base-->>GitHub: return content or 404
GitHub-->>Action: base content / not-found
Action->>Action: compute diffs, generateAutoModeSummary
Action->>GitHub: post PR comment (if PR context & enabled)
else unsafe or no valid comparePath
Action->>Action: skip base fetch, mark as new, emit warning
end
alt reportAnnotations enabled
Action->>GitHub: addAnnotations(results)
end
Action-->>Client: Complete
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 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. Comment |
PR Compliance Guide 🔍Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label |
||||||||||||||||||||||||
PR Code Suggestions ✨Explore these optional code suggestions:
|
||||||||||||||
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Greptile OverviewGreptile SummaryThis PR improves the GitHub Action's auto mode to align with explicit mode behavior and fixes base comparison logic to use output artifacts instead of source files. Key Changes:
Implementation Details:
The changes are well-structured, maintain backward compatibility, and include proper error handling and security validation. Confidence Score: 5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Action as GitHub Action
participant Minify as runMinification/runAutoMode
participant Compare as compareWithBase
participant Normalize as normalizeComparePath
participant GitHub as GitHub API
participant Report as Reporters
Action->>Minify: Execute minification
Minify->>Minify: Store outputFile (repo-relative)
Minify-->>Action: MinifyResult with outputFile
alt In PR Context
Action->>Compare: compareWithBase(result, token)
loop For each file
Compare->>Normalize: normalizeComparePath(outputFile)
Normalize-->>Compare: Validated path or null
alt outputFile invalid
Compare->>Normalize: normalizeComparePath(sourceFile)
Normalize-->>Compare: Fallback path or null
end
alt comparePath exists
Compare->>GitHub: getContent(comparePath, baseBranch)
GitHub-->>Compare: Base file size
Compare->>Compare: Calculate size delta
else comparePath unsafe
Compare->>Compare: Mark as new file
end
end
Compare-->>Action: ComparisonResult[]
end
alt reportSummary
Action->>Report: generateAutoModeSummary/generateSummary
end
alt reportPRComment
Action->>Report: postPRComment(result, comparisons)
end
alt reportAnnotations
Action->>Report: addAnnotations(result)
end
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Additional Comments (2)
Prompt To Fix With AIThis is a comment left during a code review.
Path: packages/action/src/compare.ts
Line: 41:55
Comment:
**Repo vs filesystem path**
`compareWithBase` passes `fileResult.outputFile ?? fileResult.file` directly to `octokit.rest.repos.getContent({ path })` (repo-relative path). In explicit mode `runMinification` populates `outputFile` from the `output` input (which can be an absolute/OS-specific filesystem path after resolution), so the GitHub API call will fail and the comparison will be reported as `isNew: true` (dropping base deltas). This breaks “vs base” reporting whenever users provide absolute outputs or Windows-style paths.
How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix With AIThis is a comment left during a code review.
Path: packages/action/src/comment.ts
Line: 94:103
Comment:
**Comparison key mismatch**
`generateCommentBody` builds `comparisonMap` keyed by `ComparisonResult.file` and looks up with `f.file`. After this PR, explicit mode `runMinification` sets `FileResult.file` to the *input* path and `outputFile` to the output path, while `compareWithBase` queries base using `outputFile`. If any caller still produces comparisons keyed by output paths (or mixed fixtures), the “vs Base” column will show `-` even though comparisons exist. Consider making the keying consistent (e.g., always key comparisons by `FileResult.file` or by `outputFile`, but not a mix).
How can I resolve this? If you propose a fix, please make it concise. |
Store output files as repository-relative paths and sanitize/normalize compare paths before calling the GitHub content API. This hardens base-branch comparisons against unsafe traversal/absolute paths, handles Windows separators, and preserves auto/explicit behavior with fallback logic and added tests.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/action/__tests__/runAutoMode.test.ts`:
- Line 60: Replace the unsafe cast "(context as any).payload = ..." with a
proper typed cast that declares payload shape (e.g. "(context as { payload:
Record<string, unknown> }).payload = ...") wherever it occurs in
runAutoMode.test.ts (both at the current occurrence and the one at the other
location), so tests retain type safety consistent with compare.test.ts; update
both assignments to use the typed cast rather than "as any".
🧹 Nitpick comments (2)
packages/action/src/index.ts (1)
240-249: Theas FileResult & { compressor: string }cast is loose.The
compressorfield added here is never consumed downstream sinceallResultsis typedFileResult[]. If this per-file compressor label is needed, consider adding it to theFileResulttype intypes.ts. If not, the cast can be simplified to justFileResult.packages/action/src/minify.ts (1)
24-32: ExtracttoRepositoryPathto a shared utility module to eliminate duplication.The function is defined identically in both
minify.ts(line 30) andindex.ts(line 67). Move it to a shared utility file (e.g.,utils.ts) and import it in both files.
Replace remaining `(context as any).payload` assignments in runAutoMode tests with typed payload casts to satisfy review feedback and keep test type safety.
Add compareWithBase coverage for the branch where both output and source paths are unsafe, ensuring the fallback warning+isNew path is exercised.
|
@greptile |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #2807 +/- ##
===========================================
+ Coverage 95.09% 95.18% +0.09%
===========================================
Files 73 73
Lines 1693 1725 +32
Branches 509 523 +14
===========================================
+ Hits 1610 1642 +32
Misses 83 83 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
1 participant
User description
Summary
Testing
PR Type
Bug fix, Enhancement, Tests
Description
Track output file paths in minification results for accurate base comparisons
Compare minified sizes against base output artifacts instead of source files
Scope esbuild/yui type validation to explicit mode only, allowing auto mode flexibility
Align auto mode reporting with explicit mode by honoring PR comments and annotations
Add comprehensive tests for output path tracking and auto mode reporting features
Diagram Walkthrough
File Walkthrough
3 files
Add optional outputFile field to FileResultTrack output file path in minification resultsImplement auto mode reporting with PR comments and annotations2 files
Compare against output paths instead of source filesScope type validation to explicit mode only4 files
Test output file path comparison functionalityVerify outputFile field in minification resultsTest esbuild type validation in auto modeTest auto mode reporting with comments and annotationsSummary by cubic
Aligns auto mode with explicit mode and hardens base comparisons. Auto mode now uses its own summary, can post PR comments and annotations, and compares size deltas against base output artifacts.
Written for commit 7c834bd. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes / Improvements
Tests