## Issue
Closes#5683
## Change
`GitHubDocumentLoader.fromGitHub()` wrapped an `IOException` using an
SLF4J `{}` placeholder. `RuntimeException(String, Throwable)` never
substitutes `{}`, so the message leaked a literal `{}` and dropped the
failing file's URL. The fix concatenates `content.getHtmlUrl()`,
matching the sibling message on line 210:
```java
throw new RuntimeException("Failed to load document from GitHub: " + content.getHtmlUrl(), ioException);
```
Control flow, return value, and exception type are unchanged; backward
compatible.
Added `GitHubDocumentLoaderTest` (the module's first unit test): it
injects a mock `GitHub` via the `GitHubDocumentLoader(GitHub)`
constructor, forces `GHContent.read()` to throw, and asserts the message
contains the URL and not `{}` (positive and negative).
## General checklist
- [X] There are no breaking changes (API, behaviour)
- [X] I have added unit and/or integration tests for my change
- [X] The tests cover both positive and negative cases
- [X] I have manually run all the unit and integration tests in the
module I have added/changed, and they are all green
- [ ] I have manually run all the unit and integration tests in the core
and main modules, and they are all green <!-- N/A: change isolated to
github document-loader module -->
- [ ] I have added/updated the documentation <!-- N/A: no public
API/behavior contract change -->
- [ ] I have added an example in the examples repo <!-- N/A: bug fix,
not a feature -->
- [ ] I have added/updated Spring Boot starter(s) <!-- N/A: not
applicable -->
## Build / test evidence
- `./mvnw -pl document-loaders/langchain4j-document-loader-github -am
clean test` (JDK 17): GitHubDocumentLoaderTest 1 run, 0 failures. IT
skipped (`@EnabledIfEnvironmentVariable` GITHUB_TOKEN absent).
- Spotless verified via MAIN_ROOT `spotless:apply` (jgit `ratchetFrom`
cannot open a git worktree's file-based `.git`): no reformatting of
either file.
### Summary
This PR enhances the `loadDocument` method documentation and test cases
to support loading a file from GitHub using a Git reference that can be
a **branch name**, **tag**, or **commit SHA**.
### Changes
- Renamed parameter `branch` to `ref` for better clarity and
flexibility.
- Tested support for tags and commit SHAs in addition to branch names.
- Improved parameter validation (`owner`, `repo`).
- Enhanced Javadoc with detailed usage instructions, supported ref
types, and error handling.
- Added/updated unit tests to cover:
- Valid branch
- Valid tag
- Valid commit SHA
- Invalid refs and paths
- Null or blank `ref` (defaults to repository’s default branch)
### Motivation
GitHub's API supports fetching file contents using any valid Git ref.
Updating this method to leverage that capability makes it more robust
and applicable in broader version control scenarios.
### Impact
- No breaking changes; behavior is backward compatible as `branch` usage
still works when passed via `ref`.
- Improves the flexibility and reusability of the `loadDocument` method.
### Example Usage
```java
Document doc = loader.loadDocument("langchain4j", "langchain4j", "v1.0.0", "README.md", new TextDocumentParser());
---------
Co-authored-by: Harikrishna <harikrishna.gurram@walmart.com>
## Issue
Fixes https://github.com/langchain4j/langchain4j/issues/2918
## Change
- Changed `maxRetry` parameter semantics from "max attempts" to "max
retries".
- Changed default value of the `maxRetry` parameter from 3 to 2, but it
does not change the default behaviour. When `maxRetries` parameter is
not specified explicitly, it will attempt to execute up to 3 times (as
it was before).
## Breaking Change
If you do **_not_** specify `maxRetries` parameter explicitly, there is
no breaking change and you do not need to do any changes to your code.
If you specify `maxRetries` parameter explicitly, you will need to
reduce it by 1, example:
```java
// before
OpenAiChatModel.builder()
.apiKey(System.getenv("OPENAI_API_KEY"))
.modelName(GPT_4_O_MINI)
.maxRetries(1)
.build();
// after
OpenAiChatModel.builder()
.apiKey(System.getenv("OPENAI_API_KEY"))
.modelName(GPT_4_O_MINI)
.maxRetries(0)
.build();
```
## General checklist
- [ ] There are no breaking changes
- [ ] I have added unit and/or integration tests for my change
- [ ] The tests cover both positive and negative cases
- [X] I have manually run all the unit and integration tests in the
module I have added/changed, and they are all green
- [X] I have manually run all the unit and integration tests in the
[core](https://github.com/langchain4j/langchain4j/tree/main/langchain4j-core)
and
[main](https://github.com/langchain4j/langchain4j/tree/main/langchain4j)
modules, and they are all green
- [X] I have added/updated the
[documentation](https://github.com/langchain4j/langchain4j/tree/main/docs/docs)
- [ ] I have added an example in the [examples
repo](https://github.com/langchain4j/langchain4j-examples) (only for
"big" features)
- [ ] I have added/updated [Spring Boot
starter(s)](https://github.com/langchain4j/langchain4j-spring) (if
applicable)
This is small refactoring
There are bunch of places where use deprecated methods.
These changes fix this issue.
## General checklist
<!-- Please double-check the following points and mark them like this:
[X] -->
- [x] There are no breaking changes
- [ ] I have added unit and integration tests for my change
- [x] I have manually run all the unit and integration tests in the
module I have added/changed, and they are all green
- [x] I have manually run all the unit and integration tests in the
[core](https://github.com/langchain4j/langchain4j/tree/main/langchain4j-core)
and
[main](https://github.com/langchain4j/langchain4j/tree/main/langchain4j)
modules, and they are all green
<!-- Before adding documentation and example(s) (below), please wait
until the PR is reviewed and approved. -->
- [ ] I have added/updated the
[documentation](https://github.com/langchain4j/langchain4j/tree/main/docs/docs)
- [ ] I have added an example in the [examples
repo](https://github.com/langchain4j/langchain4j-examples) (only for
"big" features)
Hi! Noticed _almost all_ tests used AssertJ, but in some cases JUnit was
still used. In addition to that some tests don't use the most expressive
assertions. Figured clean that up such that you get better assertions if
any tests were to fail. Compare for instance
```diff
- assertThat(document.metadata().asMap().size()).isEqualTo(4);
+ assertThat(document.metadata().asMap()).hasSize(4);
```
The first one will print expected 5 to be equal to 4, whereas the second
one shows the contents of the map involved.
Being consistent with your test library also stops bad patterns from
repeating accidentally through copy-and-paste. If you want to enforce
these best practices through an automated pull request check that's also
an option. Let me know if you'd want that as well. Hope that helps!
Eunbin Son