feat: emit actual koreader device / device ID#2025
Open
imnotjames wants to merge 2 commits into
Open
Conversation
Contributor
Walkthrough
ChangesKOReader device metadata
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
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 |
imnotjames
changed the title
7 tasks
imnotjames
force-pushed
the
fix/2013/koreader-device
branch
2 times, most recently
from
July 18, 2026 18:24
272e880 to
c0c3be5
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@backend/src/test/java/org/booklore/service/KoreaderServiceTest.java`:
- Around line 145-147: Replace the newly added JUnit assertEquals calls in
KoreaderServiceTest with AssertJ assertThat(...).isEqualTo(...) assertions for
both device and device_id checks, including the additional assertions at the
referenced test location.
- Around line 133-167: Extend the KoreaderServiceTest coverage around
getProgress_includesDefaultDeviceId and getProgress_includesSpecifiedDeviceId
with separate cases where only the device or only the device ID is populated,
asserting the missing field independently falls back to "Grimmory" while
preserving the supplied value. Include null and empty-string inputs for each
field where supported by the contract, using distinct progress fixtures and
matching expected values.
🪄 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: Repository YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: e4d4f41e-ff7a-4cc9-a98d-1797e217b612
📒 Files selected for processing (2)
backend/src/main/java/org/booklore/service/koreader/KoreaderService.javabackend/src/test/java/org/booklore/service/KoreaderServiceTest.java
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
grimmory-tools/grimmory-docs(manual)
📜 Review details
🧰 Additional context used
📓 Path-based instructions (5)
backend/src/**/*.java
📄 CodeRabbit inference engine (AGENTS.md)
backend/src/**/*.java: Use 4-space indentation and match surrounding Java style in backend code
Prefer constructor injection via Lombok patterns already used in the codebase. Do not introduce@Autowiredfield injection in backend code
Use MapStruct for entity/DTO mapping in backend code
Keep JPA entities on the *Entity suffix in backend code
Files:
backend/src/main/java/org/booklore/service/koreader/KoreaderService.javabackend/src/test/java/org/booklore/service/KoreaderServiceTest.java
**/*
⚙️ CodeRabbit configuration file
**/*: This project is being developed using current and future-facing technologies:
- Java 25 with --enable-preview (preview features are INTENTIONAL and encouraged)
- Spring Boot 4 (latest major version, check APIs accordingly)
- Jackson 3 (new package: tools.jackson.* instead of com.fasterxml.jackson.*)
- Hibernate 7.3.x (Jakarta Persistence 3.2, new APIs; avoid deprecated Hibernate 5/6 patterns)
- Angular 21 (signals-based reactivity, no NgModules unless legacy)
Grimmory Internal Tools
- epub4j and pdfium4j are our own internal tools developed by the Grimmory team.
- Always verify behavior and API changes against the upstream repositories:
- If you encounter issues with these libraries, check if a fix exists in the upstream grimmory-tools organization.
Metadata Standards and Compliance
- For all metadata writing and parsing logic, double-check against Dublin Core and ANSI standards to ensure perfect official compliance.
- We strictly follow the widespread and official XML-compliant methods for EPUB2, EPUB3, CBX, and PDF formats.
General Java and Spring rules
- ALWAYS prefer modern, idiomatic Java 25 constructs over legacy patterns.
- Preview features (--enable-preview) are enabled and intentional; do NOT flag them as risky unless there is a concrete runtime issue.
- Prefer: records, sealed classes/interfaces, pattern matching (switch expressions, instanceof), structured concurrency (StructuredTaskScope), scoped values, string templates, unnamed patterns/variables.
- Prefer virtual threads (Thread.ofVirtual(), Executors.newVirtualThreadPerTaskExecutor()) over platform threads for I/O-bound work.
- Prefer the new Sequenced Collections API (SequencedCollection, SequencedMap) where applicable.
- Prefer
varfor local variables when the type is obvious from context.- Use stream().toList() instead of stream().collect(Collectors.toList()) for imm...
Files:
backend/src/main/java/org/booklore/service/koreader/KoreaderService.javabackend/src/test/java/org/booklore/service/KoreaderServiceTest.java
**/service/**/*.java
⚙️ CodeRabbit configuration file
**/service/**/*.java: Spring Framework 7 service layer review:
- Flag missing
@Transactionalon methods that perform multiple writes.- Prefer constructor injection over
@Autowiredfield injection. Use@AllArgsConstructor.- Use ApiError enum for throwing exceptions.
- Flag checked exceptions swallowed silently; must log or rethrow.
- Flag Thread.sleep(); prefer Duration-based overloads or ScheduledExecutorService.
- Prefer virtual threads (Thread.ofVirtual()) for I/O-bound operations.
- Flag mutable shared state in singleton beans.
Files:
backend/src/main/java/org/booklore/service/koreader/KoreaderService.javabackend/src/test/java/org/booklore/service/KoreaderServiceTest.java
backend/src/test/**/*.java
📄 CodeRabbit inference engine (AGENTS.md)
Prefer focused unit tests; use
@SpringBootTestonly when the Spring context is required in backend code
Files:
backend/src/test/java/org/booklore/service/KoreaderServiceTest.java
**/*Test.java
⚙️ CodeRabbit configuration file
**/*Test.java: Java test review:
- Prefer
@ExtendWith(SpringExtension.class) or@SpringBootTestfor integration tests.- Flag tests with no assertions.
- Flag Thread.sleep() in tests; use Awaitility or virtual-thread-friendly alternatives.
- Flag hardcoded ports or file paths.
- Flag missing edge case coverage: null, empty, boundary values.
- Prefer AssertJ over JUnit's built-in assertions for readability.
- Prefer
@Sqlor Testcontainers for database state; not hand-rolled setup/teardown.
Files:
backend/src/test/java/org/booklore/service/KoreaderServiceTest.java
🧠 Learnings (13)
📚 Learning: 2026-04-10T08:15:37.436Z
Learnt from: imnotjames
Repo: grimmory-tools/grimmory PR: 449
File: booklore-api/src/main/java/org/booklore/service/book/BookDownloadService.java:139-145
Timestamp: 2026-04-10T08:15:37.436Z
Learning: When using Spring `ContentDisposition.builder(...).filename(name, StandardCharsets.UTF_8).build()` (i.e., explicitly providing UTF-8), the resulting header value should include both the quoted `filename="=?UTF-8?..."` and the RFC 5987 `filename*=` parameters. In this case, any extra ASCII fallback computation (e.g., deriving an ASCII `fallbackFilename` via `NON_ASCII_PATTERN` and calling `.filename(fallbackFilename)`) is likely redundant—prefer calling only `.filename(fallbackName?, StandardCharsets.UTF_8)` as appropriate and let Spring handle the UTF-8 header parameters. Verify by comparing the emitted header for `filename` and `filename*` before deciding to keep an ASCII fallback.
Applied to files:
backend/src/main/java/org/booklore/service/koreader/KoreaderService.javabackend/src/test/java/org/booklore/service/KoreaderServiceTest.java
📚 Learning: 2026-04-14T12:43:08.698Z
Learnt from: balazs-szucs
Repo: grimmory-tools/grimmory PR: 502
File: booklore-api/src/main/java/org/booklore/service/reader/ChapterCacheService.java:0-0
Timestamp: 2026-04-14T12:43:08.698Z
Learning: For this codebase (booklore-api), target Java 25 with `--enable-preview`, so `_` is intentionally used as an unnamed/ignored variable (e.g., lambda parameter or pattern variable) per Java’s preview feature JEP 456. Do not flag `_` in those contexts as an invalid/reserved identifier; only flag it if it’s used in a non-supported position (e.g., where an unnamed variable is not applicable for the Java preview rules).
Applied to files:
backend/src/main/java/org/booklore/service/koreader/KoreaderService.javabackend/src/test/java/org/booklore/service/KoreaderServiceTest.java
📚 Learning: 2026-05-07T21:21:55.233Z
Learnt from: imnotjames
Repo: grimmory-tools/grimmory PR: 1194
File: backend/src/main/java/org/booklore/service/ReadingSessionService.java:0-0
Timestamp: 2026-05-07T21:21:55.233Z
Learning: When reviewing Java 23+ code, treat `java.time.Instant#until(Instant endExclusive)` as a valid API/method call (it returns a `Duration`, equivalent to `Duration.between(this, endExclusive)`). Do not flag `instant.until(otherInstant)` as a compile error or API misuse when the project targets Java 25+ (as in grimmory-tools/grimmory); the call should be considered correct and returns a `Duration`.
Applied to files:
backend/src/main/java/org/booklore/service/koreader/KoreaderService.javabackend/src/test/java/org/booklore/service/KoreaderServiceTest.java
📚 Learning: 2026-05-08T06:19:20.621Z
Learnt from: imnotjames
Repo: grimmory-tools/grimmory PR: 1201
File: backend/src/main/java/org/booklore/model/dto/AccessTokenDto.java:3-10
Timestamp: 2026-05-08T06:19:20.621Z
Learning: For Jackson 3 codebases, do not treat imports from `com.fasterxml.jackson.annotation.*` (e.g., `JsonInclude`, `JsonProperty`, `JsonView`) as incorrect. In Jackson 3, `jackson-annotations` intentionally remains under `com.fasterxml.jackson.annotation.*` for backward compatibility, while only the core processing packages (e.g., `jackson-core`, `jackson-databind`) move to the `tools.jackson.*` namespace.
Applied to files:
backend/src/main/java/org/booklore/service/koreader/KoreaderService.javabackend/src/test/java/org/booklore/service/KoreaderServiceTest.java
📚 Learning: 2026-05-04T20:31:11.075Z
Learnt from: imnotjames
Repo: grimmory-tools/grimmory PR: 1086
File: backend/src/main/java/org/booklore/service/metadata/BookReviewUpdateService.java:63-66
Timestamp: 2026-05-04T20:31:11.075Z
Learning: For this repository, reviewers should treat string truncation done via `String.length()` and `String.substring(0, maxLength)` (UTF-16 code units) as an accepted, consistent convention. Do not flag individual occurrences of this pattern as bugs, even though it is not code-point-aware for surrogate pairs. A separate global effort is already tracked to move toward code-point-aware truncation, so per-site fixes should be avoided during code review.
Applied to files:
backend/src/main/java/org/booklore/service/koreader/KoreaderService.java
📚 Learning: 2026-05-13T12:34:49.607Z
Learnt from: balazs-szucs
Repo: grimmory-tools/grimmory PR: 1293
File: backend/src/main/java/org/booklore/service/metadata/DuckDuckGoCoverService.java:246-252
Timestamp: 2026-05-13T12:34:49.607Z
Learning: In this repo’s Java code, when catching Jsoup `org.jsoup.HttpStatusException` (and similar exceptions originating from external libraries) and wrapping/rethrowing them, do not require preserving the original exception stack trace (e.g., as flagged by PMD `PreserveStackTrace`) as long as the application already captures the actionable diagnostics in logs or the thrown exception message (such as HTTP status code and the requested URL). Reviewers should still ensure the log/message contains those details; the intent is to avoid noisy stack traces that only reflect external-library internals rather than application code.
Applied to files:
backend/src/main/java/org/booklore/service/koreader/KoreaderService.java
📚 Learning: 2026-05-17T13:38:16.462Z
Learnt from: balazs-szucs
Repo: grimmory-tools/grimmory PR: 1366
File: backend/src/main/java/org/booklore/service/FileStreamingService.java:65-66
Timestamp: 2026-05-17T13:38:16.462Z
Learning: In the grimmory-tools/grimmory repo, it’s an accepted pattern to pass the raw AccessDeniedException.getMessage() (even if it may include filesystem path details) into ApiError.PERMISSION_DENIED.createException(...). During code review, do not raise a security/information-disclosure issue solely based on that exception message being propagated to the API when using ApiError.PERMISSION_DENIED.createException with the AccessDeniedException message.
Applied to files:
backend/src/main/java/org/booklore/service/koreader/KoreaderService.java
📚 Learning: 2026-05-23T23:01:25.769Z
Learnt from: imnotjames
Repo: grimmory-tools/grimmory PR: 1456
File: backend/src/main/java/org/booklore/service/metadata/parser/GoodReadsParser.java:618-630
Timestamp: 2026-05-23T23:01:25.769Z
Learning: In this codebase (grimmory-tools/grimmory), it’s intentional to omit per-request timeouts on individual Java HttpRequest.Builder instances (e.g., GoodReadsParser.fetchJson). During reviews, do not flag missing builder-level timeouts as a best-practice violation; rely on framework-level and/or HttpClient-level timeouts configured elsewhere for consistent behavior. Only raise an issue if you can verify that no effective timeout is configured at the HttpClient/framework level.
Applied to files:
backend/src/main/java/org/booklore/service/koreader/KoreaderService.java
📚 Learning: 2026-06-12T01:10:31.416Z
Learnt from: imnotjames
Repo: grimmory-tools/grimmory PR: 1724
File: backend/src/main/java/org/booklore/repository/BookRepository.java:58-59
Timestamp: 2026-06-12T01:10:31.416Z
Learning: In this codebase (grimmory-tools/grimmory), reviews should not treat inline `LIMIT`/`OFFSET` clauses inside `Query` JPQL/HQL strings as a JPA compliance risk. This is intentional: `hibernate.jpa.compliance.query=true` is intentionally not set, and Hibernate 7.3+ supports `LIMIT`/`OFFSET` as valid HQL extensions. Therefore, do not flag or require changes to `Query` annotations solely due to `LIMIT`/`OFFSET` usage.
Applied to files:
backend/src/main/java/org/booklore/service/koreader/KoreaderService.java
📚 Learning: 2026-05-07T21:37:46.988Z
Learnt from: imnotjames
Repo: grimmory-tools/grimmory PR: 1194
File: backend/src/main/java/org/booklore/service/ReadingSessionService.java:121-123
Timestamp: 2026-05-07T21:37:46.988Z
Learning: In grimmory-tools/grimmory service-layer code, if arithmetic overflow occurs inside inference/business-logic (e.g., when deriving inferred fields like durationSeconds from start/end timestamps), treat it as a server-side anomaly. Prefer letting the global exception handler translate it into a generic 5xx response rather than throwing an explicit ApiError 4xx (e.g., do not convert overflow into a client error).
Applied to files:
backend/src/main/java/org/booklore/service/koreader/KoreaderService.java
📚 Learning: 2026-04-27T15:25:55.042Z
Learnt from: imnotjames
Repo: grimmory-tools/grimmory PR: 930
File: backend/src/test/java/org/booklore/service/metadata/parser/ComicvineBookParserTest.java:68-75
Timestamp: 2026-04-27T15:25:55.042Z
Learning: In this repository’s JUnit 5 test sources (e.g., under backend/src/test/java/), do not flag “bare” `assert` statements as a bug. The project test runner is configured to always execute tests with assertions enabled (e.g., `-ea`), so `assert` behavior is consistent. Continue to review for correctness, but don’t treat unguarded `assert` usage in test classes as a static-analysis issue.
Applied to files:
backend/src/test/java/org/booklore/service/KoreaderServiceTest.java
📚 Learning: 2026-05-22T03:20:45.559Z
Learnt from: imnotjames
Repo: grimmory-tools/grimmory PR: 1446
File: backend/src/test/java/org/booklore/service/metadata/MetadataManagementServiceTest.java:465-470
Timestamp: 2026-05-22T03:20:45.559Z
Learning: In backend unit tests (Java files under backend/src/test/java), do not flag hardcoded placeholder filesystem paths (e.g., setting LibraryPathEntity.path = "/example") as violations when the value is intentionally unused for filesystem access. Only suppress the "no hardcoded paths" concern if the test does not perform any filesystem/network IO using that path (no reads/writes/Files.* calls or code paths that access the filesystem with that value); if the placeholder is actually used to touch the filesystem, flag it.
Applied to files:
backend/src/test/java/org/booklore/service/KoreaderServiceTest.java
📚 Learning: 2026-05-04T05:01:33.919Z
Learnt from: imnotjames
Repo: grimmory-tools/grimmory PR: 1081
File: backend/src/test/java/org/booklore/service/metadata/parser/AudibleParserTest.java:132-135
Timestamp: 2026-05-04T05:01:33.919Z
Learning: In grimmory-tools/grimmory unit/integration test classes (e.g., files matching **/*Test.java under backend/src/test/java/**), it is acceptable to directly instantiate Jackson mappers (e.g., `new ObjectMapper()` / `new JsonMapper(...)`) and this should NOT be flagged. The Jackson guidance to use Spring bean injection or `JsonMapper.shared()` applies only to production code; tests may construct dependencies directly as standard practice. Production-code mapper instantiation rules should still be enforced outside test sources.
Applied to files:
backend/src/test/java/org/booklore/service/KoreaderServiceTest.java
🔇 Additional comments (1)
backend/src/main/java/org/booklore/service/koreader/KoreaderService.java (1)
61-75: LGTM!
imnotjames
force-pushed
the
fix/2013/koreader-device
branch
from
July 18, 2026 21:31
1b96395 to
00cdc5c
Compare
imnotjames
marked this pull request as ready for review
July 18, 2026 21:31
Contributor
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 `@backend/src/main/java/org/booklore/service/koreader/KoreaderService.java`:
- Around line 61-67: Update the device and deviceId fallback logic in
KoreaderService.java (lines 61-67) to treat null, empty, and whitespace-only
values as absent, preserving the existing defaults. Add coverage in
KoreaderServiceTest.java (lines 169-195) asserting empty and blank device inputs
fall back to "Grimmory" and the corresponding default device ID.
🪄 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: Repository YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: 726c355b-af76-4b2e-a48f-cf91f6b3a771
📒 Files selected for processing (2)
backend/src/main/java/org/booklore/service/koreader/KoreaderService.javabackend/src/test/java/org/booklore/service/KoreaderServiceTest.java
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
grimmory-tools/grimmory-docs(manual)
📜 Review details
⏰ Context from checks skipped due to timeout. (5)
- GitHub Check: Test Suite / Frontend Tests
- GitHub Check: Test Suite / Backend Tests
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (java-kotlin)
- GitHub Check: Frontend Lint Threshold Check
🧰 Additional context used
📓 Path-based instructions (5)
backend/src/**/*.java
📄 CodeRabbit inference engine (AGENTS.md)
backend/src/**/*.java: Use 4-space indentation and match surrounding Java style in backend code
Prefer constructor injection via Lombok patterns already used in the codebase. Do not introduce@Autowiredfield injection in backend code
Use MapStruct for entity/DTO mapping in backend code
Keep JPA entities on the *Entity suffix in backend code
Files:
backend/src/test/java/org/booklore/service/KoreaderServiceTest.javabackend/src/main/java/org/booklore/service/koreader/KoreaderService.java
backend/src/test/**/*.java
📄 CodeRabbit inference engine (AGENTS.md)
Prefer focused unit tests; use
@SpringBootTestonly when the Spring context is required in backend code
Files:
backend/src/test/java/org/booklore/service/KoreaderServiceTest.java
**/*
⚙️ CodeRabbit configuration file
**/*: This project is being developed using current and future-facing technologies:
- Java 25 with --enable-preview (preview features are INTENTIONAL and encouraged)
- Spring Boot 4 (latest major version, check APIs accordingly)
- Jackson 3 (new package: tools.jackson.* instead of com.fasterxml.jackson.*)
- Hibernate 7.3.x (Jakarta Persistence 3.2, new APIs; avoid deprecated Hibernate 5/6 patterns)
- Angular 21 (signals-based reactivity, no NgModules unless legacy)
Grimmory Internal Tools
- epub4j and pdfium4j are our own internal tools developed by the Grimmory team.
- Always verify behavior and API changes against the upstream repositories:
- If you encounter issues with these libraries, check if a fix exists in the upstream grimmory-tools organization.
Metadata Standards and Compliance
- For all metadata writing and parsing logic, double-check against Dublin Core and ANSI standards to ensure perfect official compliance.
- We strictly follow the widespread and official XML-compliant methods for EPUB2, EPUB3, CBX, and PDF formats.
General Java and Spring rules
- ALWAYS prefer modern, idiomatic Java 25 constructs over legacy patterns.
- Preview features (--enable-preview) are enabled and intentional; do NOT flag them as risky unless there is a concrete runtime issue.
- Prefer: records, sealed classes/interfaces, pattern matching (switch expressions, instanceof), structured concurrency (StructuredTaskScope), scoped values, string templates, unnamed patterns/variables.
- Prefer virtual threads (Thread.ofVirtual(), Executors.newVirtualThreadPerTaskExecutor()) over platform threads for I/O-bound work.
- Prefer the new Sequenced Collections API (SequencedCollection, SequencedMap) where applicable.
- Prefer
varfor local variables when the type is obvious from context.- Use stream().toList() instead of stream().collect(Collectors.toList()) for imm...
Files:
backend/src/test/java/org/booklore/service/KoreaderServiceTest.javabackend/src/main/java/org/booklore/service/koreader/KoreaderService.java
**/service/**/*.java
⚙️ CodeRabbit configuration file
**/service/**/*.java: Spring Framework 7 service layer review:
- Flag missing
@Transactionalon methods that perform multiple writes.- Prefer constructor injection over
@Autowiredfield injection. Use@AllArgsConstructor.- Use ApiError enum for throwing exceptions.
- Flag checked exceptions swallowed silently; must log or rethrow.
- Flag Thread.sleep(); prefer Duration-based overloads or ScheduledExecutorService.
- Prefer virtual threads (Thread.ofVirtual()) for I/O-bound operations.
- Flag mutable shared state in singleton beans.
Files:
backend/src/test/java/org/booklore/service/KoreaderServiceTest.javabackend/src/main/java/org/booklore/service/koreader/KoreaderService.java
**/*Test.java
⚙️ CodeRabbit configuration file
**/*Test.java: Java test review:
- Prefer
@ExtendWith(SpringExtension.class) or@SpringBootTestfor integration tests.- Flag tests with no assertions.
- Flag Thread.sleep() in tests; use Awaitility or virtual-thread-friendly alternatives.
- Flag hardcoded ports or file paths.
- Flag missing edge case coverage: null, empty, boundary values.
- Prefer AssertJ over JUnit's built-in assertions for readability.
- Prefer
@Sqlor Testcontainers for database state; not hand-rolled setup/teardown.
Files:
backend/src/test/java/org/booklore/service/KoreaderServiceTest.java
🧠 Learnings (13)
📚 Learning: 2026-04-10T08:15:37.436Z
Learnt from: imnotjames
Repo: grimmory-tools/grimmory PR: 449
File: booklore-api/src/main/java/org/booklore/service/book/BookDownloadService.java:139-145
Timestamp: 2026-04-10T08:15:37.436Z
Learning: When using Spring `ContentDisposition.builder(...).filename(name, StandardCharsets.UTF_8).build()` (i.e., explicitly providing UTF-8), the resulting header value should include both the quoted `filename="=?UTF-8?..."` and the RFC 5987 `filename*=` parameters. In this case, any extra ASCII fallback computation (e.g., deriving an ASCII `fallbackFilename` via `NON_ASCII_PATTERN` and calling `.filename(fallbackFilename)`) is likely redundant—prefer calling only `.filename(fallbackName?, StandardCharsets.UTF_8)` as appropriate and let Spring handle the UTF-8 header parameters. Verify by comparing the emitted header for `filename` and `filename*` before deciding to keep an ASCII fallback.
Applied to files:
backend/src/test/java/org/booklore/service/KoreaderServiceTest.javabackend/src/main/java/org/booklore/service/koreader/KoreaderService.java
📚 Learning: 2026-04-14T12:43:08.698Z
Learnt from: balazs-szucs
Repo: grimmory-tools/grimmory PR: 502
File: booklore-api/src/main/java/org/booklore/service/reader/ChapterCacheService.java:0-0
Timestamp: 2026-04-14T12:43:08.698Z
Learning: For this codebase (booklore-api), target Java 25 with `--enable-preview`, so `_` is intentionally used as an unnamed/ignored variable (e.g., lambda parameter or pattern variable) per Java’s preview feature JEP 456. Do not flag `_` in those contexts as an invalid/reserved identifier; only flag it if it’s used in a non-supported position (e.g., where an unnamed variable is not applicable for the Java preview rules).
Applied to files:
backend/src/test/java/org/booklore/service/KoreaderServiceTest.javabackend/src/main/java/org/booklore/service/koreader/KoreaderService.java
📚 Learning: 2026-05-07T21:21:55.233Z
Learnt from: imnotjames
Repo: grimmory-tools/grimmory PR: 1194
File: backend/src/main/java/org/booklore/service/ReadingSessionService.java:0-0
Timestamp: 2026-05-07T21:21:55.233Z
Learning: When reviewing Java 23+ code, treat `java.time.Instant#until(Instant endExclusive)` as a valid API/method call (it returns a `Duration`, equivalent to `Duration.between(this, endExclusive)`). Do not flag `instant.until(otherInstant)` as a compile error or API misuse when the project targets Java 25+ (as in grimmory-tools/grimmory); the call should be considered correct and returns a `Duration`.
Applied to files:
backend/src/test/java/org/booklore/service/KoreaderServiceTest.javabackend/src/main/java/org/booklore/service/koreader/KoreaderService.java
📚 Learning: 2026-05-08T06:19:20.621Z
Learnt from: imnotjames
Repo: grimmory-tools/grimmory PR: 1201
File: backend/src/main/java/org/booklore/model/dto/AccessTokenDto.java:3-10
Timestamp: 2026-05-08T06:19:20.621Z
Learning: For Jackson 3 codebases, do not treat imports from `com.fasterxml.jackson.annotation.*` (e.g., `JsonInclude`, `JsonProperty`, `JsonView`) as incorrect. In Jackson 3, `jackson-annotations` intentionally remains under `com.fasterxml.jackson.annotation.*` for backward compatibility, while only the core processing packages (e.g., `jackson-core`, `jackson-databind`) move to the `tools.jackson.*` namespace.
Applied to files:
backend/src/test/java/org/booklore/service/KoreaderServiceTest.javabackend/src/main/java/org/booklore/service/koreader/KoreaderService.java
📚 Learning: 2026-04-27T15:25:55.042Z
Learnt from: imnotjames
Repo: grimmory-tools/grimmory PR: 930
File: backend/src/test/java/org/booklore/service/metadata/parser/ComicvineBookParserTest.java:68-75
Timestamp: 2026-04-27T15:25:55.042Z
Learning: In this repository’s JUnit 5 test sources (e.g., under backend/src/test/java/), do not flag “bare” `assert` statements as a bug. The project test runner is configured to always execute tests with assertions enabled (e.g., `-ea`), so `assert` behavior is consistent. Continue to review for correctness, but don’t treat unguarded `assert` usage in test classes as a static-analysis issue.
Applied to files:
backend/src/test/java/org/booklore/service/KoreaderServiceTest.java
📚 Learning: 2026-05-22T03:20:45.559Z
Learnt from: imnotjames
Repo: grimmory-tools/grimmory PR: 1446
File: backend/src/test/java/org/booklore/service/metadata/MetadataManagementServiceTest.java:465-470
Timestamp: 2026-05-22T03:20:45.559Z
Learning: In backend unit tests (Java files under backend/src/test/java), do not flag hardcoded placeholder filesystem paths (e.g., setting LibraryPathEntity.path = "/example") as violations when the value is intentionally unused for filesystem access. Only suppress the "no hardcoded paths" concern if the test does not perform any filesystem/network IO using that path (no reads/writes/Files.* calls or code paths that access the filesystem with that value); if the placeholder is actually used to touch the filesystem, flag it.
Applied to files:
backend/src/test/java/org/booklore/service/KoreaderServiceTest.java
📚 Learning: 2026-05-04T05:01:33.919Z
Learnt from: imnotjames
Repo: grimmory-tools/grimmory PR: 1081
File: backend/src/test/java/org/booklore/service/metadata/parser/AudibleParserTest.java:132-135
Timestamp: 2026-05-04T05:01:33.919Z
Learning: In grimmory-tools/grimmory unit/integration test classes (e.g., files matching **/*Test.java under backend/src/test/java/**), it is acceptable to directly instantiate Jackson mappers (e.g., `new ObjectMapper()` / `new JsonMapper(...)`) and this should NOT be flagged. The Jackson guidance to use Spring bean injection or `JsonMapper.shared()` applies only to production code; tests may construct dependencies directly as standard practice. Production-code mapper instantiation rules should still be enforced outside test sources.
Applied to files:
backend/src/test/java/org/booklore/service/KoreaderServiceTest.java
📚 Learning: 2026-05-04T20:31:11.075Z
Learnt from: imnotjames
Repo: grimmory-tools/grimmory PR: 1086
File: backend/src/main/java/org/booklore/service/metadata/BookReviewUpdateService.java:63-66
Timestamp: 2026-05-04T20:31:11.075Z
Learning: For this repository, reviewers should treat string truncation done via `String.length()` and `String.substring(0, maxLength)` (UTF-16 code units) as an accepted, consistent convention. Do not flag individual occurrences of this pattern as bugs, even though it is not code-point-aware for surrogate pairs. A separate global effort is already tracked to move toward code-point-aware truncation, so per-site fixes should be avoided during code review.
Applied to files:
backend/src/main/java/org/booklore/service/koreader/KoreaderService.java
📚 Learning: 2026-05-13T12:34:49.607Z
Learnt from: balazs-szucs
Repo: grimmory-tools/grimmory PR: 1293
File: backend/src/main/java/org/booklore/service/metadata/DuckDuckGoCoverService.java:246-252
Timestamp: 2026-05-13T12:34:49.607Z
Learning: In this repo’s Java code, when catching Jsoup `org.jsoup.HttpStatusException` (and similar exceptions originating from external libraries) and wrapping/rethrowing them, do not require preserving the original exception stack trace (e.g., as flagged by PMD `PreserveStackTrace`) as long as the application already captures the actionable diagnostics in logs or the thrown exception message (such as HTTP status code and the requested URL). Reviewers should still ensure the log/message contains those details; the intent is to avoid noisy stack traces that only reflect external-library internals rather than application code.
Applied to files:
backend/src/main/java/org/booklore/service/koreader/KoreaderService.java
📚 Learning: 2026-05-17T13:38:16.462Z
Learnt from: balazs-szucs
Repo: grimmory-tools/grimmory PR: 1366
File: backend/src/main/java/org/booklore/service/FileStreamingService.java:65-66
Timestamp: 2026-05-17T13:38:16.462Z
Learning: In the grimmory-tools/grimmory repo, it’s an accepted pattern to pass the raw AccessDeniedException.getMessage() (even if it may include filesystem path details) into ApiError.PERMISSION_DENIED.createException(...). During code review, do not raise a security/information-disclosure issue solely based on that exception message being propagated to the API when using ApiError.PERMISSION_DENIED.createException with the AccessDeniedException message.
Applied to files:
backend/src/main/java/org/booklore/service/koreader/KoreaderService.java
📚 Learning: 2026-05-23T23:01:25.769Z
Learnt from: imnotjames
Repo: grimmory-tools/grimmory PR: 1456
File: backend/src/main/java/org/booklore/service/metadata/parser/GoodReadsParser.java:618-630
Timestamp: 2026-05-23T23:01:25.769Z
Learning: In this codebase (grimmory-tools/grimmory), it’s intentional to omit per-request timeouts on individual Java HttpRequest.Builder instances (e.g., GoodReadsParser.fetchJson). During reviews, do not flag missing builder-level timeouts as a best-practice violation; rely on framework-level and/or HttpClient-level timeouts configured elsewhere for consistent behavior. Only raise an issue if you can verify that no effective timeout is configured at the HttpClient/framework level.
Applied to files:
backend/src/main/java/org/booklore/service/koreader/KoreaderService.java
📚 Learning: 2026-06-12T01:10:31.416Z
Learnt from: imnotjames
Repo: grimmory-tools/grimmory PR: 1724
File: backend/src/main/java/org/booklore/repository/BookRepository.java:58-59
Timestamp: 2026-06-12T01:10:31.416Z
Learning: In this codebase (grimmory-tools/grimmory), reviews should not treat inline `LIMIT`/`OFFSET` clauses inside `Query` JPQL/HQL strings as a JPA compliance risk. This is intentional: `hibernate.jpa.compliance.query=true` is intentionally not set, and Hibernate 7.3+ supports `LIMIT`/`OFFSET` as valid HQL extensions. Therefore, do not flag or require changes to `Query` annotations solely due to `LIMIT`/`OFFSET` usage.
Applied to files:
backend/src/main/java/org/booklore/service/koreader/KoreaderService.java
📚 Learning: 2026-05-07T21:37:46.988Z
Learnt from: imnotjames
Repo: grimmory-tools/grimmory PR: 1194
File: backend/src/main/java/org/booklore/service/ReadingSessionService.java:121-123
Timestamp: 2026-05-07T21:37:46.988Z
Learning: In grimmory-tools/grimmory service-layer code, if arithmetic overflow occurs inside inference/business-logic (e.g., when deriving inferred fields like durationSeconds from start/end timestamps), treat it as a server-side anomaly. Prefer letting the global exception handler translate it into a generic 5xx response rather than throwing an explicit ApiError 4xx (e.g., do not convert overflow into a client error).
Applied to files:
backend/src/main/java/org/booklore/service/koreader/KoreaderService.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
properly emits the expected koreader device / device ID - with default fallbacks to
Grimmory. this will allow koreader users to be less confused when trying to accept progress from remoteLinked Issue
fixes #2013
Changes
Manual Testing Steps
Grimmoryand is instead the name of the ereader that last pushed progressScreenshots (Optional)
Additional Context (Optional)
AI Disclosure
None
Checklist
just ui checkandjust api check.Summary by CodeRabbit