Skip to content

refactor: move core into llama/ submodule under a Maven reactor (parent + llama + llama-langchain4j)#288

Draft
bernardladenthin wants to merge 6 commits into
mainfrom
claude/reactor-core-into-llama-submodule
Draft

refactor: move core into llama/ submodule under a Maven reactor (parent + llama + llama-langchain4j)#288
bernardladenthin wants to merge 6 commits into
mainfrom
claude/reactor-core-into-llama-submodule

Conversation

@bernardladenthin

Copy link
Copy Markdown
Owner

Summary

  • Turns the repo into a Maven reactor. The root is now a thin aggregator/parent POM (net.ladenthin:llama-parent, packaging=pom) with two modules: llama/ (the native JNI core, moved here wholesale) and llama-langchain4j/. The core keeps its published coordinates net.ladenthin:llama — consumers are unaffected.
  • Version lockstep by construction. Both modules inherit the single <version> from the parent, so the previous CI version-lockstep guard is gone. A single reactor mvn -P release deploy signs and publishes all three artifacts (parent pom, llama, llama-langchain4j) at the same version (the release profile — GPG + Central Publishing — moved to the parent; the redundant separate llama-langchain4j deploy steps are removed).
  • Native build re-rooted with one hook. .github/build.sh / build.bat cd into llama/ relative to the script's own location, so every native build (incl. the build_cuda_linux.sh / build_opencl_*.sh wrappers and the dockcross containers, whose workdir stays the repo root) configures the module — no per-job working-directory gymnastics.
  • Paths re-rooted across publish.yml (mvn -f llama/pom.xml, ctest --test-dir llama/build, all artifact + webui-generated paths, GIT_TAG from llama/CMakeLists.txt, jacoco/pit/vmlens/jar produce-paths), clang-format.yml, sonarqube.yml, REUSE.toml, .gitignore. codeql.yml stays a root reactor compile (now also covering llama-langchain4j).

Test plan

  • Affected unit / integration tests pass locally — full reactor builds; mvn -pl llama -am install (the langchain4j jobs' install step) installs parent + core; mvn -N -P release validate parses the moved release profile; mvn -f llama-langchain4j/pom.xml verify is green (7 mapping tests, 4 self-skip, main+sources+javadoc jars).
  • CI is green on this branch — the native build/test matrix (dockcross, macOS, Windows, CUDA/OpenCL/Vulkan) exercises the re-rooted paths and can only be validated in the pipeline; this is the point of the draft. Expect one or more path fix-ups.
  • Docs updated — CLAUDE.md gains a "Repository layout — Maven reactor" section (incl. the caveat that existing root-relative build examples now run in llama/) and a reactor-accurate LangChain4j section.

Related issues / PRs

Notes for review

  • Draft on purpose. The structural + Maven/Java side is locally verified; the native-CI path re-rooting is not locally testable and is what this run validates. I'll iterate on any red job.
  • No behavioural change to the native build — only paths and the reactor wiring. The core artifact net.ladenthin:llama is unchanged for consumers.

Checklist

  • My commits follow Conventional Commits
  • No security-sensitive changes

🤖 Generated with Claude Code


Generated by Claude Code

claude added 3 commits July 1, 2026 19:51
…ven reactor)

Phase 2 structural move. The repo root becomes a thin aggregator/parent POM
(net.ladenthin:llama-parent, packaging=pom) with two modules: llama (the native
JNI core) and llama-langchain4j. Both inherit the single shared <version> from
the parent, so they ship in lockstep by construction (the CI version-lockstep
guard becomes redundant and is removed in the CI pass).

- git mv the entire core into llama/: src/, CMakeLists.txt, cmake/, patches/,
  pom.xml, spotbugs-exclude.xml, lombok.config, .clang-format, .clang-tidy.
  (All tracked as renames; the core pom's ${basedir}/… and relative
  spotbugs-exclude.xml auto-re-root; CMakeLists paths are relative to itself.)
- New root pom.xml = aggregator (modules: llama, llama-langchain4j).
- llama/pom.xml and llama-langchain4j/pom.xml become children of llama-parent
  (drop their own <version>; inherit it). The core keeps its published
  coordinates net.ladenthin:llama — consumers are unaffected by the move.

Verified locally: the reactor builds green (parent → llama → llama-langchain4j),
all three install at 5.0.4-SNAPSHOT, langchain4j resolves the core via the
inherited ${project.version}.

CI path re-rooting (workflows, build scripts, REUSE.toml, .gitignore) follows in
the next commit.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Rt1paYztGJ2AKUuBuAGDXE
…, workflows, configs)

Follows the structural move: everything that assumed the core lived at the repo
root now points into the llama/ module. No behavioural change to the native
build — only paths.

- build.sh / build.bat: cd into llama/ (relative to the script's own location)
  before cmake, so every native build (incl. the build_cuda_linux.sh /
  build_opencl_*.sh wrappers and the dockcross containers, whose workdir stays the
  repo root) configures the module. This single hook re-roots all native builds.
- publish.yml: mvn compile/test/PIT/vmlens/spotbugs/spotless now target
  -f llama/pom.xml; ctest uses llama/build; all artifact upload/download +
  webui-generated paths prefixed with llama/; GIT_TAG read from llama/CMakeLists.txt;
  jacoco/pit/vmlens/jar produce-paths -> llama/target (report-job download dirs
  stay at root). Removed the now-redundant version-lockstep guard (inheritance
  guarantees it) and the separate llama-langchain4j deploy steps (one reactor
  `mvn -P release deploy` publishes parent + llama + llama-langchain4j together).
- release profile (GPG + Central Publishing) moved to the parent pom so the reactor
  deploy signs every module incl. the parent .pom; removed from the two child poms.
- clang-format.yml, sonarqube.yml, REUSE.toml, .gitignore re-rooted to llama/.
  codeql.yml left as a root reactor compile (now also covers llama-langchain4j).

Verified locally: full reactor builds; `mvn -pl llama -am install` (the langchain4j
jobs' install step) installs parent+core; `mvn -N -P release validate` parses the
moved release profile; `mvn -f llama-langchain4j verify` is green (7 mapping tests,
4 self-skip, main+sources+javadoc jars). Native path re-rooting is validated by CI.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Rt1paYztGJ2AKUuBuAGDXE
- New "Repository layout — Maven reactor" section: the root is an aggregator/parent
  POM with llama/ (core) and llama-langchain4j/ modules; core sources + build files
  now live under llama/; build.sh/build.bat cd into llama/ themselves; core Maven
  builds use -f llama/pom.xml or -pl llama -am; version bumps happen in the root pom
  only (both modules inherit). Adds the caveat that existing root-relative build
  examples in this file now run in llama/.
- Rewrote the LangChain4j section: it is now a reactor module inheriting the parent
  version (no lockstep guard), releasing via one reactor deploy (inherited release
  profile), with the install step using -pl llama -am.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Rt1paYztGJ2AKUuBuAGDXE
Comment thread pom.xml
<version>${reactor.version}</version>
<scope>test</scope>
</dependency>
<profiles>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent design: the parent POM holds the release profile centrally, so both child modules inherit GPG + Central Publishing configuration. A single mvn -P release deploy publishes all three artifacts (parent pom + llama + llama-langchain4j) at the same version. This eliminates the CI version-lockstep guard and duplicate profile definitions.

Comment thread .github/build.sh
# cmake configures the module regardless of the caller's CWD. Anchored to this script's
# own location (via BASH_SOURCE), so it works from the repo root and inside the dockcross
# container whose workdir is the mounted repo root.
cd "$(dirname "${BASH_SOURCE[0]}")/../llama" || exit 1

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clean solution: the build script anchors itself to its own location via BASH_SOURCE, so it works from the repo root and inside dockcross containers (whose workdir stays the repo root). This single cd re-roots all native builds without per-job working-directory gymnastics. The comment is helpful for understanding the intent.

Comment thread llama-langchain4j/pom.xml
<modelVersion>4.0.0</modelVersion>

<groupId>net.ladenthin</groupId>
<parent>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good: llama-langchain4j now correctly declares llama-parent as its parent, inheriting the version and release profile. The relativePath points to ../pom.xml. This eliminates the CI version-lockstep guard that was previously needed. ✓

# :llama-langchain4j. The `release` profile (GPG + Central Publishing) is inherited
# from the parent, so every module — including the parent pom — is signed.
- name: Publish snapshot (reactor - parent + llama + llama-langchain4j)
run: mvn --batch-mode --no-transfer-progress -P release,cuda,opencl-android,windows-msvc,cuda-windows,vulkan-windows,opencl-windows -Dmaven.test.skip=true deploy

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect: the snapshot deploy is now a single reactor mvn -P release deploy, which publishes all three artifacts (parent pom + llama + llama-langchain4j) in one command. The release profile is inherited from the parent, so every module is signed. This replaces the previous separate llama-langchain4j deploy step. ✓

@claude

claude Bot commented Jul 1, 2026

Copy link
Copy Markdown

This is a well-executed Maven reactor refactoring. The structural changes are clean and significantly improve the project layout. No behavioral changes to the native build or consumer API — the core artifact net.ladenthin:llama remains unchanged.

Strengths:

  1. Parent POM design — The llama-parent aggregator holds the shared release profile, so one reactor mvn -P release deploy signs and publishes all artifacts at the same version. Eliminates the CI version-lockstep guard.

  2. Native build re-rooting — The cd in build.sh / build.bat anchors to the script itself, so it works from the repo root and inside dockcross containers without per-job working-directory changes. A single hook re-roots all native builds.

  3. Path migration is consistent — Every workflow step, artifact path, and config file now correctly points to llama/src/, llama/CMakeLists.txt, etc.

  4. Version management simplified — Both modules inherit <version> from the parent. Bump root pom.xml only; both modules follow automatically.

  5. llama-langchain4j wiring — Now a proper reactor module with llama-parent as parent. Version lockstep guaranteed by construction.

  6. Documentation — CLAUDE.md comprehensively documents the new layout with caveat that build examples run in llama/.

Status:

  • Structural + Maven/Java locally verified ✓
  • Native CI path re-rooting will be validated in pipeline (draft status is appropriate)
  • All commits follow Conventional Commits ✓
  • No security issues ✓

Well-executed. All path updates consistent and correct. Ready for CI validation of the native build matrix. 👍

…ter the llama/ move

The C++ compile failed on every native job with `fatal error: jni.h: No such
file or directory`. The bundled JNI headers live in .github/include/ at the repo
ROOT, but CMakeLists.txt now runs inside the llama/ module, so the relative
`.github/include/unix` resolved to the non-existent llama/.github/include/unix.
Anchor it to ${CMAKE_CURRENT_SOURCE_DIR}/../.github/include/{unix,windows} so it
points at the repo root regardless of build CWD. Covers Linux, macOS and Windows
(all use the bundled headers). The other CMakeLists paths use
${CMAKE_SOURCE_DIR}/… which already resolves inside llama/.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Rt1paYztGJ2AKUuBuAGDXE
Pre-existing formatting violation in special_tokens_json (the token-id map added
by #286 / ModelMeta) that this branch inherited unchanged through the move. The
reactor PR's clang-format check surfaced it. Reflowed with the pinned
clang-format 22.1.5; no logic change. (Implies main currently carries the same
violation — it self-resolves when this merges.)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Rt1paYztGJ2AKUuBuAGDXE
@claude

claude Bot commented Jul 1, 2026

Copy link
Copy Markdown

PR Review: Maven Reactor Refactoring - Documentation Issue

Critical Issue: CLAUDE.md documentation contradicts the actual code structure.

The PR moves code into a Maven reactor with an aggregator parent and two modules (llama, llama-langchain4j), BUT the CLAUDE.md removed the "Repository layout" section and reverted LangChain4j description to "standalone sibling module" — which contradicts the actual POM structure.

Current code reality:

  • Root pom.xml defines <modules> with both llama and llama-langchain4j
  • Both inherit version and release profile from parent
  • One reactor deploy publishes all three artifacts

CLAUDE.md says:

  • LangChain4j is "standalone sibling module, deliberately not in the root reactor"
  • No reactor layout section explaining the new structure

This must be fixed: Either restore the reactor layout docs to match the code, or revert the code to actual standalone. The PR description states reactor structure, so restore CLAUDE.md to document it.

Comment thread llama/CMakeLists.txt
# dir so the path is correct regardless of the build CWD.
if(OS_NAME MATCHES "^Linux" OR OS_NAME STREQUAL "Mac" OR OS_NAME STREQUAL "Darwin")
set(JNI_INCLUDE_DIRS .github/include/unix)
set(JNI_INCLUDE_DIRS ${CMAKE_CURRENT_SOURCE_DIR}/../.github/include/unix)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Good fix: anchoring JNI headers to repo root via ${CMAKE_CURRENT_SOURCE_DIR}/../.github/include/ ensures cmake resolves them correctly regardless of build CWD. This would have failed all native builds before the fix.

Comment thread .github/build.sh
# cmake configures the module regardless of the caller's CWD. Anchored to this script's
# own location (via BASH_SOURCE), so it works from the repo root and inside the dockcross
# container whose workdir is the mounted repo root.
cd "$(dirname "${BASH_SOURCE[0]}")/../llama" || exit 1

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Good: cd "$(dirname "${BASH_SOURCE[0]}")/../llama" re-roots the build into the llama/ module relative to the script's own location. This single hook re-roots every native build (incl. build_cuda_linux.sh / build_opencl_*.sh wrappers and dockcross containers).

Comment thread .github/build.bat
REM The core project (CMakeLists.txt + src\) lives in the `llama\` module of the Maven
REM reactor. Re-root here once (relative to this script's own location) so cmake
REM configures the module regardless of the caller's CWD.
cd /d "%~dp0..\llama" || exit /b 1

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Good: Windows equivalent of build.sh — cd /d "%~dp0..\llama" re-roots into llama/ module. Both Unix and Windows scripts now have the same single re-rooting hook.

Comment thread pom.xml
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<project.build.outputTimestamp>${git.commit.time}</project.build.outputTimestamp>
</properties>
<modules>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Correct aggregator structure: root POM declares both llama and llama-langchain4j modules, so they build and deploy together in lockstep.

Comment thread llama/pom.xml
<parent>
<groupId>net.ladenthin</groupId>
<artifactId>llama-parent</artifactId>
<version>5.0.4-SNAPSHOT</version>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Correct parent reference: <relativePath>../pom.xml</relativePath> points to the aggregator parent. Both llama and llama-langchain4j inherit the single <version> from the parent, guaranteeing lockstep by construction.

Comment thread CLAUDE.md
@@ -1225,51 +1225,76 @@ keeping it clear of the JPMS module-mode javadoc trap that bit BAF. **Before rai
javadoc source level to ≥ 9, read**
[`../workspace/policies/jpms-module-descriptor.md`](../workspace/policies/jpms-module-descriptor.md).

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Documentation inconsistency: This section was deleted, but the code now IS a Maven reactor with llama-langchain4j as a module in the root pom.xml. The removed "Repository layout — Maven reactor" section should be restored to document the new aggregator structure, the module layout, and updated build command examples (e.g., core cmake builds now run in llama/). Current state: CLAUDE.md says "standalone sibling, deliberately not in the root reactor" but the code has it in <modules>.

Comment thread CLAUDE.md
## LangChain4j integration (`llama-langchain4j` sibling module)
## Repository layout — Maven reactor (`llama/` + `llama-langchain4j/`)

The repo root is a thin **aggregator/parent POM** (`net.ladenthin:llama-parent`,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ This description contradicts the actual code structure. The PR moved llama-langchain4j INTO the reactor as a module (see root pom.xml <modules>), so it's no longer "deliberately not in the root reactor." Update this to clarify: "It is now a reactor module alongside the core llama module" and reference the restored "Repository layout" section above.

@claude

claude Bot commented Jul 1, 2026

Copy link
Copy Markdown

Summary of Review

Code Quality: EXCELLENT - No bugs found in Maven POM configuration, native build re-rooting, or script changes. The CMake JNI header path anchoring is correct.

Documentation: CRITICAL ISSUE - The code implements a reactor structure but CLAUDE.md contradicts it. CLAUDE.md says llama-langchain4j is a standalone module not in the reactor, but it is actually listed in the root pom.xml modules. The 'Repository layout' section needs to be restored to document the aggregator structure.

Copy link
Copy Markdown
Owner Author

On the flagged CLAUDE.md "contradiction": this is a misread of the diff — the documentation already matches the reactor code, so there's nothing to fix here.

At the current branch HEAD (71b3859):

  • CLAUDE.md:1228 — the ## Repository layout — Maven reactor (llama/ + llama-langchain4j/) section exists (aggregator parent, both modules, version inheritance, and the "core cmake builds run in llama/" caveat).
  • CLAUDE.md:1257 — the section heading is ## LangChain4j integration (llama-langchain4j reactor module), and line 1261 reads "It is a reactor module alongside the core llama module."
  • grep -n "standalone sibling\|deliberately \*not\* in the root reactor" CLAUDE.mdno matches.

The standalone sibling module, deliberately not in the root reactor text the review quotes is the removed (-) side of the diff (the old pre-reactor wording), not the current file. The rewrite to reactor-accurate docs landed in the same commit that introduced the reactor. No documentation change is needed.


Generated by Claude Code

@claude

claude Bot commented Jul 1, 2026

Copy link
Copy Markdown

Code Review Summary

This PR restructures the repository into a Maven reactor with parent POM and two modules (llama and llama-langchain4j), improving organization and version management.

Strengths:

  1. Version lockstep guaranteed at model level, eliminating CI guards
  2. Comprehensive path migration in build scripts and workflows
  3. No breaking changes to consumers (published coordinates unchanged)
  4. Clear documentation of new layout

Issues Found:

CRITICAL: CLAUDE.md outdated build examples
Lines 35-38 still reference root pom.xml in CUDA upgrade example. Should be llama/pom.xml.

Lines 800+ in Build Commands section contain many root-relative paths (cmake -B build, mvn compile) that are now inaccurate since core moved to llama/ module. The clarifying note explaining to interpret root examples as running in llama/ was removed but examples remain unchanged.

MINOR: Confirm version-lockstep validation in CI workflow actually fails the build if module version diverges from parent version.

Security: No sensitive files exposed. GPG/release profile properly migrated.

Recommendation: Hold for CLAUDE.md fixes - update build examples to either reference llama/ explicitly or restore the clarifying note about interpreting root paths as running in llama/.

Comment thread CLAUDE.md
Comment on lines +1243 to +1253

**Consequences for build commands:** the core's cmake/native build runs *in `llama/`*.
`.github/build.sh` / `build.bat` `cd` into `llama/` themselves (relative to the script), so CI
and the dockcross containers (whose workdir stays the repo root) are unaffected. Locally, run
core cmake builds from `llama/` (e.g. `cd llama && cmake -B build && cmake --build build`), and
target the core with Maven via `-f llama/pom.xml` (or `-pl llama -am` from the root). A plain
`mvn` at the root builds the whole reactor. **When a build-command example elsewhere in this
file shows `cmake -B build` / `src/main/...` / `mvn compile` at the root, read it as running in
`llama/`** (the paths moved; the recipes are otherwise unchanged).

**Version bump:** change the `<version>` in the **root** `pom.xml` only; `llama` and

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Documentation issue: The CI section here describes checking version lockstep: "runs a version-lockstep guard (module version must equal core version, else the build fails — the standalone module can't inherit ${project.version} from a reactor)".

However, the langchain4j module uses ${project.version} from the parent (which it inherits). Can you clarify or point to where the lockstep validation actually happens in the CI workflow? Is this an explicit check, or does it rely on Maven's implicit enforcement?

Comment thread CLAUDE.md
Comment on lines +1245 to +1252
`.github/build.sh` / `build.bat` `cd` into `llama/` themselves (relative to the script), so CI
and the dockcross containers (whose workdir stays the repo root) are unaffected. Locally, run
core cmake builds from `llama/` (e.g. `cd llama && cmake -B build && cmake --build build`), and
target the core with Maven via `-f llama/pom.xml` (or `-pl llama -am` from the root). A plain
`mvn` at the root builds the whole reactor. **When a build-command example elsewhere in this
file shows `cmake -B build` / `src/main/...` / `mvn compile` at the root, read it as running in
`llama/`** (the paths moved; the recipes are otherwise unchanged).

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clarification needed: The description says the llama-langchain4j module "Carries its own sources/javadoc/gpg + release profile" and "the module has no parent to inherit them from — plugin versions are pinned in lockstep with the root pom.xml".

However, the POM clearly shows:

<parent>
  <groupId>net.ladenthin</groupId>
  <artifactId>llama-parent</artifactId>
  <version>5.0.4-SNAPSHOT</version>
  <relativePath>../pom.xml</relativePath>
</parent>

This contradicts "the module has no parent". The module DOES have a parent (llama-parent), so it DOES inherit the version. Please update this section to accurately reflect the new reactor structure where both modules inherit from the parent.

Comment thread CLAUDE.md
Comment on lines +1253 to 1260
**Version bump:** change the `<version>` in the **root** `pom.xml` only; `llama` and
`llama-langchain4j` inherit it. (The SNAPSHOT/`-SNAPSHOT` line and the README badge still need
the usual manual update.)

## LangChain4j integration (`llama-langchain4j` reactor module)

`llama-langchain4j/` adapts a `LlamaModel` to LangChain4j's `ChatModel`,
`StreamingChatModel`, `EmbeddingModel` and `ScoringModel` interfaces **in-process over

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Documentation accuracy issue: The description here says the langchain4j CI job "runs a version-lockstep guard (module version must equal core version, else the build fails — the standalone module can't inherit ${project.version} from a reactor)".

However, looking at the actual workflow in publish.yml:

  1. Line 170-172: The module is installed via mvn -pl llama -am ... install (parent + core only)
  2. Line 174: Then mvn -f llama-langchain4j/pom.xml verify builds the module in isolation

There's no explicit version-lockstep validation shown here. The version is inherited from the parent via the parent reference in the module's POM, so versions are automatically in sync — there's no CI guard needed. Please clarify whether an explicit guard is supposed to exist, or update the docs to say version lockstep is guaranteed by construction (parent inheritance) rather than by a CI guard.

Comment thread llama/pom.xml
<relativePath>../pom.xml</relativePath>
</parent>

<artifactId>llama</artifactId>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note for future reference: While the module POM correctly references the parent, consumers should be aware that build examples in CLAUDE.md (like cmake commands, path references) now apply to the llama/ directory. The published coordinates remain net.ladenthin:llama, so consuming code is unaffected, but internal contributors should run cmake -B build from within the llama/ directory (the .github/build.sh and .github/build.bat scripts handle this automatically).

@claude

claude Bot commented Jul 1, 2026

Copy link
Copy Markdown

Review Complete

I've identified and commented on the documentation issues. To summarize:

Main Issues (all in CLAUDE.md):

  1. Lines 1245-1252: The description contradicts itself - it says the module "has no parent" but the POM clearly has a parent reference. The new reactor structure should be clearly documented here.

  2. Lines 1253-1260: Confusing wording about version-lockstep guard. Since the module inherits from the parent, versions are locked by construction, not by a CI guard. This should be clarified.

  3. Build examples in other sections (not modified in this PR): Many root-relative paths like src/main/cpp/ and src/test/cpp/ are still used throughout CLAUDE.md but should now refer to llama/src/.... These weren't changed in this PR but should be updated in a follow-up to avoid confusing contributors.

Positive observations:

✅ Solid architectural design - version lockstep guaranteed at model level
✅ Comprehensive CI path updates - all workflow references correctly point to llama/ module
✅ Build scripts properly handle re-rooting via relative paths
✅ Published coordinates unchanged - consumers unaffected
✅ Reactor deploy workflow correctly handles parent + both modules

The structural work is solid. Once you clarify the CLAUDE.md documentation (particularly fixing the contradictions about the parent relationship and version lockstep), this should be ready for CI validation.

…DE.md

The per-section upgrade instructions (CUDA/Android/OpenCL/Windows classifier
steps and the llama.cpp version bump) name the core `pom.xml` / `CMakeLists.txt`
that a contributor would copy-paste — those now live in llama/. Re-rooted the
precise `**pom.xml**` / `**CMakeLists.txt**` callouts, the `# Edit …` lines, the
`git add …` lines, and the build-webui GIT_TAG reference to llama/. The **root**
aggregator pom.xml (the version-bump target) is deliberately left unqualified.
The loose build-command examples (cmake -B build / mvn compile) remain covered by
the "read root examples as running in llama/" note in the Repository layout section.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Rt1paYztGJ2AKUuBuAGDXE
Comment thread CLAUDE.md
Comment on lines +1232 to +1233

- **`llama/`** — the native JNI core (`net.ladenthin:llama`). *All the core sources and build

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Documentation bug: This section contains contradictions:

  • Lines 1232-1233 claim llama-langchain4j is "deliberately not in the root reactor"
  • Lines 1250 claim "the module has no parent"
  • Lines 1254-1255 claim "the standalone module can't inherit ${project.version} from a reactor"

However, the actual implementation shows:

  1. Root pom.xml lists it as a module: <module>llama-langchain4j</module>
  2. llama-langchain4j/pom.xml declares the parent: <parent><artifactId>llama-parent</artifactId>
  3. It inherits the version: <version>5.0.4-SNAPSHOT</version> is inherited from the parent

Per the PR description, llama-langchain4j IS now part of the Maven reactor. The docs need to be corrected to accurately reflect this architectural change.

Comment thread CLAUDE.md
Comment on lines +1253 to +1256
**Version bump:** change the `<version>` in the **root** `pom.xml` only; `llama` and
`llama-langchain4j` inherit it. (The SNAPSHOT/`-SNAPSHOT` line and the README badge still need
the usual manual update.)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Documentation inconsistency: The docs claim there is a "version-lockstep guard" that "module version must equal core version, else the build fails." However:

  1. The CI job test-java-llama-langchain4j (lines 158-175 of publish.yml) has no version guard — it just runs mvn -f llama-langchain4j/pom.xml verify
  2. Both modules now inherit ${project.version} from the parent, so version lockstep is guaranteed by construction, not by CI validation
  3. The old guard is "gone" (per line 154-155 of publish.yml), but CLAUDE.md still describes it as if it exists

The docs should state: "Version lockstep is guaranteed by construction (both modules inherit the parent's version), so no explicit CI guard is needed."

@sonarqubecloud

sonarqubecloud Bot commented Jul 1, 2026

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
78.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants