Add Matrix metadata for LINE images#189
Conversation
|
@coderabbitai review |
|
Warning Review limit reached
More reviews will be available in 13 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughReplaces hardcoded ChangesDynamic LINE image format detection
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Billing warning: we have not been able to collect payment for this subscription for more than 72 hours. Please update the payment method or pay any pending invoices in Billing to avoid service interruption. 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 |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
pkg/connector/handlers/image.go (2)
180-198: ⚡ Quick winConsider logging the fallback to image/jpeg.
When the MIME type cannot be detected, the function silently defaults to
image/jpeg. Logging this fallback would improve observability and help identify unexpected formats from LINE.📊 Proposed enhancement for observability
If you add a logger parameter (or refactor to a method), you could log the fallback:
} } + // Fallback to JPEG; log for observability + // (Consider adding logger param to enable this) return "image/jpeg" }Alternatively, consider returning an explicit indicator (like a second bool return value) when using the fallback, so the caller can log it.
🤖 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 `@pkg/connector/handlers/image.go` around lines 180 - 198, The detectLineImageMimeType function silently falls back to returning "image/jpeg" when the MIME type cannot be detected, which reduces observability. Either add a logger parameter to the function and log when the fallback to "image/jpeg" occurs (just before the final return statement), or refactor the function to return both the MIME type string and a boolean flag indicating whether a fallback was used, so the caller can log it. Choose the approach that best fits your codebase's error handling patterns.
172-175: ⚡ Quick winConsider logging DecodeConfig failures for observability.
The error from
image.DecodeConfigis silently ignored. While the fallback behavior (omitting dimensions) is safe, logging the error would help diagnose format support issues, especially for formats like HEIC/HEIF that may not decode with the standard library.📊 Proposed enhancement for observability
if config, _, err := image.DecodeConfig(bytes.NewReader(data)); err == nil { info.Width = config.Width info.Height = config.Height + } else { + // Log decode failure for observability; dimensions will be omitted + // Note: HEIC/HEIF and some formats may not have stdlib decoder support + h.Log.Debug(). + Err(err). + Str("mime_type", mimeType). + Msg("Could not decode image dimensions") }Note: This would require passing the logger (h.Log) to the function or refactoring to a method.
🤖 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 `@pkg/connector/handlers/image.go` around lines 172 - 175, The error returned from image.DecodeConfig is being silently ignored, which prevents observability into why image dimensions cannot be decoded, especially for formats like HEIC/HEIF. When the err returned from image.DecodeConfig is not nil, log this error using the logger (h.Log) instead of silently ignoring it. This may require refactoring the function to accept the logger as a parameter or converting the logic to a method that has access to h.Log.
🤖 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.
Nitpick comments:
In `@pkg/connector/handlers/image.go`:
- Around line 180-198: The detectLineImageMimeType function silently falls back
to returning "image/jpeg" when the MIME type cannot be detected, which reduces
observability. Either add a logger parameter to the function and log when the
fallback to "image/jpeg" occurs (just before the final return statement), or
refactor the function to return both the MIME type string and a boolean flag
indicating whether a fallback was used, so the caller can log it. Choose the
approach that best fits your codebase's error handling patterns.
- Around line 172-175: The error returned from image.DecodeConfig is being
silently ignored, which prevents observability into why image dimensions cannot
be decoded, especially for formats like HEIC/HEIF. When the err returned from
image.DecodeConfig is not nil, log this error using the logger (h.Log) instead
of silently ignoring it. This may require refactoring the function to accept the
logger as a parameter or converting the logic to a method that has access to
h.Log.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1ce4ce9b-63a9-4f62-8088-d30b9a5537f1
📒 Files selected for processing (3)
.gitignorepkg/connector/handlers/image.gopkg/connector/handlers/image_test.go
💤 Files with no reviewable changes (1)
- .gitignore
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: Usego fmtfor code formatting across all Go files
Usegoimportswith-local "github.com/highesttt/matrix-line-messenger"flag to group project-local imports correctly
Usezerologfor logging throughout the codebase
Do not useMsgfin logging; useMsgwith structured fields instead
UseStringerinterface where applicable in Go code
Files:
pkg/connector/handlers/image.gopkg/connector/handlers/image_test.go
**/!(ltsm)/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/!(ltsm)/**/*.go: Runstaticcheckon all Go files excludingpkg/ltsmpackage (transpiled WASM code)
Rungo veton all Go files excludingpkg/ltsmpackage (transpiled WASM code)
Files:
pkg/connector/handlers/image.gopkg/connector/handlers/image_test.go
🔇 Additional comments (10)
pkg/connector/handlers/image.go (6)
4-22: LGTM!
116-118: LGTM!
147-148: LGTM!
154-163: LGTM!
200-217: LGTM!
1-243: Verify compliance with coding guidelines — Unable to complete verification.The review comment requests verification that this file passes
staticcheck,go vet, andgoimportschecks as specified in the coding guidelines. The initial linting verification failed due to a network error and could not be completed.However, from code inspection, the file does comply with zerolog usage requirements: all logging calls use
.Msg()with structured fields (.Str(),.Bool(),.Dur(),.Err(),.Int()) and do not use.Msgf(). Stringer interface is not applicable to this file.Complete verification of
go vet,staticcheck, andgoimportscompliance requires running these tools directly.pkg/connector/handlers/image_test.go (4)
15-78: LGTM!
80-98: LGTM!
100-128: LGTM!
1-129: Manual verification required: staticcheck, go vet, and goimports compliance.Repository access is currently unavailable for automated verification. Please ensure this test file passes the following checks as required by the coding guidelines:
go vet ./pkg/connector/handlers/image_test.gostaticcheck ./pkg/connector/handlers/image_test.gogoimports -local "github.com/highesttt/matrix-line-messenger" -d ./pkg/connector/handlers/image_test.go
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR improves Matrix image message conversion by detecting the actual image MIME type, generating an appropriate filename/extension, and attaching FileInfo (including dimensions when decodable), with new tests to validate the behavior.
Changes:
- Add
lineImageMediaInfoto detect MIME type, compute filename/extension, and populateevent.FileInfo(size + optional dimensions). - Factor message content creation into
lineImageEventContentto consistently attachInfoand preserverelates_to. - Add Go tests for JPEG/PNG dimension extraction and HEIC/fallback MIME detection; update
.gitignoreto allow tracking_test.gofiles.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| pkg/connector/handlers/image.go | Detect image MIME type/extension, attach FileInfo, refactor content creation. |
| pkg/connector/handlers/image_test.go | Add table-driven tests for MIME/filename/dimension behavior and content wiring. |
| .gitignore | Stop ignoring *_test.go so tests are committed. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
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 `@pkg/connector/handlers/image_test.go`:
- Line 156: The test case for image/svg+xml in the test data lacks an explicit
switch case in the imageExtensionForMIME function, making it depend on the
system's MIME type database instead of having deterministic behavior like other
image types. Either add an explicit case statement in the imageExtensionForMIME
function to handle "image/svg+xml" and return "svg" for consistency with other
image types, or remove the test case from the test data if SVG support is not a
required feature. Choose the approach based on whether SVG image handling is
needed in your application.
🪄 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: CHILL
Plan: Pro
Run ID: 14c0bbbb-25c7-4927-9d73-8d4c4e0a61de
📒 Files selected for processing (2)
pkg/connector/handlers/image.gopkg/connector/handlers/image_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- pkg/connector/handlers/image.go
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: Usego fmtfor code formatting across all Go files
Usegoimportswith-local "github.com/highesttt/matrix-line-messenger"flag to group project-local imports correctly
Usezerologfor logging throughout the codebase
Do not useMsgfin logging; useMsgwith structured fields instead
UseStringerinterface where applicable in Go code
Files:
pkg/connector/handlers/image_test.go
**/!(ltsm)/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/!(ltsm)/**/*.go: Runstaticcheckon all Go files excludingpkg/ltsmpackage (transpiled WASM code)
Rungo veton all Go files excludingpkg/ltsmpackage (transpiled WASM code)
Files:
pkg/connector/handlers/image_test.go
🔇 Additional comments (1)
pkg/connector/handlers/image_test.go (1)
46-155: LGTM!Also applies to: 157-217
|
@clins1994 this might fix your download issue on ios too? |
Summary
content.infoto bridged image events so clients can identify them as imagesFixes #155
Notes
I haven't verified this on the iOS client yet, but this fills the missing Matrix media metadata that clients normally use to identify image attachments.
Testing
GOWORK=off go test ./pkg/connector/handlersgo test ./...in Dockergo vet $(go list ./... | grep -v /ltsm)in Dockerstaticcheck $(go list ./... | grep -v /ltsm)in Dockergoimports -local "github.com/highesttt/matrix-line-messenger" -l ...in Dockerdocker build .