Skip to content

Expand snapshot publishing to non-pull request events#113

Merged
bernardladenthin merged 1 commit into
mainfrom
claude/debug-snapshot-publish-rXX7v
May 8, 2026
Merged

Expand snapshot publishing to non-pull request events#113
bernardladenthin merged 1 commit into
mainfrom
claude/debug-snapshot-publish-rXX7v

Conversation

@bernardladenthin
Copy link
Copy Markdown
Owner

Summary

Updated the conditional logic for the snapshot publishing workflow to trigger on a broader set of events beyond just push events.

Key Changes

  • Modified the publish-snapshot job condition from github.event_name == 'push' to github.event_name != 'pull_request'
  • This allows snapshot publishing to occur on push events, workflow_dispatch, and other non-pull-request triggers

Implementation Details

The change enables snapshot artifacts to be published for additional event types while still preventing publication during pull request events. This is useful for scenarios where snapshots should be published from manual workflow triggers or other repository events beyond standard push operations.

https://claude.ai/code/session_01XkYMtHTs5PKue1mm8a8sfE

Replaces the positive `push` check with a negative `pull_request`
exclusion so workflow_dispatch and release triggers also publish the
snapshot when the package job succeeds.

https://claude.ai/code/session_01XkYMtHTs5PKue1mm8a8sfE
@bernardladenthin bernardladenthin merged commit 53f307a into main May 8, 2026
1 of 10 checks passed
@bernardladenthin bernardladenthin deleted the claude/debug-snapshot-publish-rXX7v branch May 8, 2026 11:14
bernardladenthin pushed a commit that referenced this pull request May 23, 2026
Exposes llama.cpp's llama_model_params.progress_callback as a Java
functional interface. New constructor:

  new LlamaModel(parameters, progress -> { ... return true; });

The callback receives a float in [0.0, 1.0] on the loader thread
(same thread that called the constructor) and may return false to
abort, in which case the constructor throws LlamaException.

JNI: extracts the existing loadModel body into load_model_impl,
adds a trampoline that forwards float progress to a Java
LoadProgressCallback.onProgress(float)Z via CallBooleanMethod.
Trampoline state lives on the loader stack — bounded lifetime is
the single load call.

Two native entry points share the implementation:
  loadModel(String[])                              — unchanged signature
  loadModelWithProgress(String[], LoadProgressCallback)

Tests in LoadProgressCallbackTest (model-gated): non-decreasing
progress in [0,1] reaching ~1.0, returning false aborts with
LlamaException, null callback overload delegates to plain loadModel.
All 435 C++ unit tests still pass. mvn javadoc:jar BUILD SUCCESS.

https://claude.ai/code/session_01R4ZrEy3ptJDLuUgUKuM4Gy
bernardladenthin pushed a commit that referenced this pull request May 23, 2026
The kotlin-comparison doc and the open-issues doc were both stale
after PR #188 shipped 11 features. Bring them up to date in place
rather than introducing a separate changelog:

docs/feature-investigation-llama-stack-client-kotlin.md
- §1 capability matrix gets new rows for everything that landed
  (typed chat + tools, async wrappers, reactive Publisher, batch
  dispatch, Usage/Timings, Session, completeAsJson, TokenLogprob,
  CancellationToken, LoadProgressCallback).
- New §1.1 status legend (SHIPPED / PARTIAL / OPEN).
- Each §2.x section now starts with a Status: line summarising what
  shipped, with commit refs into this PR. §2.2/2.3/2.4/2.5/2.7/2.8/
  2.9/2.10 marked SHIPPED. §2.6 PARTIAL (locking deferred).
  §2.10 PARTIAL — cooperative cancel shipped; immediate cancel
  needs a new server-side JNI primitive (M-effort follow-up).
  §2.1 OPEN (multimodal image API).

docs/history/49be664_open_issues.md
- #113 updated STILL POSSIBLE -> FIXED in PR #188 commit 70df324,
  with a note that the richer payload (file name, bytes, weights
  flag) is intentionally not exposed because the upstream
  llama_model_params.progress_callback emits only a float.

No code changes, no test impact.

https://claude.ai/code/session_01R4ZrEy3ptJDLuUgUKuM4Gy
bernardladenthin added a commit that referenced this pull request May 23, 2026
* Add typed Usage / Timings / ServerMetrics accessors (§2.5)

Introduces Usage, Timings, and ServerMetrics value classes plus
LlamaModel.getMetricsTyped() so callers no longer need to parse the
raw JSON from getMetrics() by hand. Mirrors the existing ModelMeta
pattern. 15 unit tests, no native or JNI changes.

https://claude.ai/code/session_01R4ZrEy3ptJDLuUgUKuM4Gy

* Add per-request setJsonSchema + completeAsJson<T> helpers (§2.8)

InferenceParameters gains setJsonSchema(String) mirroring the existing
ModelParameters setter. LlamaModel.completeAsJson<T> sets the schema,
runs complete(), and deserializes the result via Jackson, throwing a
LlamaException if the model output is not valid JSON for the target
type. No JNI changes — the native server already accepts json_schema
in slot params.

https://claude.ai/code/session_01R4ZrEy3ptJDLuUgUKuM4Gy

* Add typed logprobs to LlamaOutput (§2.9)

New TokenLogprob record carries token text, id, raw prob/logprob, and
the nested top_probs/top_logprobs alternatives. LlamaOutput.logprobs
is populated by CompletionResponseParser.parseLogprobs from the same
completion_probabilities array that already feeds the flat
probabilities map. Existing constructor stays as a delegator so all
prior callers keep working with logprobs defaulting to empty.

https://claude.ai/code/session_01R4ZrEy3ptJDLuUgUKuM4Gy

* Document iterator cancel semantics + idempotency regression (§2.7)

LlamaIterator.cancel() / close() were already wired correctly via the
existing JNI cancelCompletion → erase_reader path, so this is purely a
docs + test pass:

- Clarify in LlamaIterator javadoc that the underlying llama.cpp slot
  may continue to its natural stop after cancel(), while the reader is
  released immediately and next() stops yielding.
- Document close() idempotency (post-natural-stop, post-cancel,
  double-close all safe).
- Add try-with-resources example to LlamaModel.generate javadoc.
- Add testIteratorCloseIdempotent in LlamaModelTest covering both the
  drained-then-closed and cancelled-then-closed paths and confirming
  the model is still usable afterwards.

https://claude.ai/code/session_01R4ZrEy3ptJDLuUgUKuM4Gy

* Add CancellationToken + complete(params, token) overload (§2.10)

CancellationToken wraps an AtomicInteger task id and a model
reference. LlamaModel.complete(params, token) runs the streaming
inference path internally, binds the token, accumulates text, and
returns early when token.cancel() is invoked from another thread.
The token is reset on return so it is reusable across calls.

No JNI changes: reuses the existing cancelCompletion native method
(which erases the JNI reader; the upstream slot completes naturally).

https://claude.ai/code/session_01R4ZrEy3ptJDLuUgUKuM4Gy

* Add CompletableFuture async wrappers for complete/chatComplete (§2.3)

LlamaModel gains completeAsync, chatCompleteAsync, and
chatCompleteTextAsync — thin wrappers that dispatch the existing
blocking methods through ForkJoinPool.commonPool(). The
completeAsync(params, token) overload bridges future.cancel(true) to
CancellationToken.cancel() so cancellation propagates into the
inference loop.

Reactive Flow.Publisher streaming (M-effort) is intentionally deferred
to a follow-up; this PR delivers only the S-effort portion of §2.3.

https://claude.ai/code/session_01R4ZrEy3ptJDLuUgUKuM4Gy

* Add Session multi-turn helper + ChatMessage value type (§2.6)

Session is a thin wrapper over LlamaModel: it owns a slot id, an
accumulating user/assistant transcript, and an optional system
message and parameter customizer. send(userMessage) appends both
sides of the turn and runs chatCompleteText with the full history.
stream(userMessage) returns a LlamaIterable for streamed replies;
commitStreamedReply records the assistant turn once the caller has
accumulated the text.

save/restore delegate to existing LlamaModel.saveSlot/restoreSlot.
close() erases the slot's KV cache.

Single-threaded use only in this pass — per-session locking is the
M-effort follow-up. ChatMessage is the minimal value type for the
transcript; will be reused by ChatResponse when §2.2 lands.

https://claude.ai/code/session_01R4ZrEy3ptJDLuUgUKuM4Gy

* Fix CI VM crash: make CancellationToken cooperative-only

Cross-thread cancel raced with the JNI receive loop: cancel() called
cancelCompletion() from another thread, which erased the underlying
server_response_reader unique_ptr while the main thread held a raw
pointer to it and was blocked inside rd->next(). On the next token
this dereferenced freed memory and aborted with std::system_error,
crashing the test JVM (exit 134).

Fix: cancel() now sets a volatile flag only. The inference loop in
complete(params, token) checks the flag between tokens and, when set,
calls cancelCompletion from the same thread that just returned from
receiveCompletionJson — safe because no concurrent access remains.

Latency becomes one token interval (tens to a few hundred ms on CPU)
instead of immediate. Documented in CancellationToken javadoc.

Tests:
- LlamaModelTest#testCompleteWithCancellationToken: budget relaxed
  from 5s to 30s (was tight even on the happy path).
- LlamaModelTest#testCompleteAsyncCancelPropagates: drop the brittle
  poll on token.isCancelled() (the worker resets the token on return
  before the assertion sees it); sleep for cancel propagation and
  verify the model is still usable.

https://claude.ai/code/session_01R4ZrEy3ptJDLuUgUKuM4Gy

* Fix javadoc build error + reduce warnings on new classes

The release packaging job (mvn package, release profile) runs
maven-javadoc-plugin's attach-javadocs which treats Javadoc tool
errors as build failures. PR #188 introduced one such error:
TokenLogprob.java had a </p> with no matching <p> (the prose was
already enclosed by an outer <p>...</p>, and the inner </p> was
stray).

Fix the error and bring my new public APIs up to a clean shape:
- TokenLogprob: rebalance the <p>/</p> HTML and add @return / @param
  to public getters and constructor.
- Timings, Usage, ServerMetrics, ChatMessage, CancellationToken,
  Session, LlamaOutput: add @return / @param tags with a leading
  one-line description (the "no main description" warning fires on
  bare /** @return ... */ blocks).
- LlamaModel: restore the doc comment for complete(params, token)
  that was accidentally stripped during an earlier edit, and add one
  for getMetricsTyped(); remove a stray orphan doc block.

Local verification:
  mvn clean javadoc:jar -DskipTests=true -Dgpg.skip=true
  mvn -P release -Dmaven.test.skip=true -Dgpg.skip=true package
Both: BUILD SUCCESS (was: BUILD FAILURE, 1 error, 100 warnings).
60 warnings remain, all from pre-existing files outside this PR.

Document the verification command and the failure categories
(errors vs warnings) in CLAUDE.md under "Javadoc — must build
cleanly before mvn package".

https://claude.ai/code/session_01R4ZrEy3ptJDLuUgUKuM4Gy

* Add LoadProgressCallback for model-load progress (#113)

Exposes llama.cpp's llama_model_params.progress_callback as a Java
functional interface. New constructor:

  new LlamaModel(parameters, progress -> { ... return true; });

The callback receives a float in [0.0, 1.0] on the loader thread
(same thread that called the constructor) and may return false to
abort, in which case the constructor throws LlamaException.

JNI: extracts the existing loadModel body into load_model_impl,
adds a trampoline that forwards float progress to a Java
LoadProgressCallback.onProgress(float)Z via CallBooleanMethod.
Trampoline state lives on the loader stack — bounded lifetime is
the single load call.

Two native entry points share the implementation:
  loadModel(String[])                              — unchanged signature
  loadModelWithProgress(String[], LoadProgressCallback)

Tests in LoadProgressCallbackTest (model-gated): non-decreasing
progress in [0,1] reaching ~1.0, returning false aborts with
LlamaException, null callback overload delegates to plain loadModel.
All 435 C++ unit tests still pass. mvn javadoc:jar BUILD SUCCESS.

https://claude.ai/code/session_01R4ZrEy3ptJDLuUgUKuM4Gy

* Add typed ChatRequest/ChatResponse + tool calling + agent loop (§2.2)

New typed chat API on top of the existing handleChatCompletions JNI
path — no native changes.

Value types:
- ChatChoice, ChatResponse  — choices array, Usage, Timings, raw JSON
- ToolCall, ToolDefinition  — OAI-shaped tool wire types
- ChatMessage (extended)    — tool_call_id + tool_calls support, with
  toolResult() and assistantToolCalls() factory methods (backwards-
  compatible 2-arg constructor kept for Session and existing tests)
- ToolHandler               — functional interface for tool callbacks
- ChatRequest               — builder with messages, tools, tool_choice,
  maxToolRounds, and an InferenceParameters customizer

InferenceParameters: new setMessagesJson(String), setToolsJson(String),
setToolChoice(String) for verbatim JSON injection from ChatRequest.

LlamaModel:
- chat(ChatRequest) → ChatResponse
  Serializes the request (auto-enables use_jinja when tools present),
  calls chatComplete, parses the OAI JSON into ChatResponse via the
  extended ChatResponseParser.parseResponse.
- chatWithTools(ChatRequest, Map<String, ToolHandler>) → ChatResponse
  Agent loop: per round, calls chat(); if the assistant returned
  tool_calls, invokes each handler (capturing exceptions as
  {"error":...} tool results so the loop continues), appends the
  assistant turn and tool-result turns to the request, and loops up to
  ChatRequest.maxToolRounds (default 8). Unknown tool names produce a
  {"error":"unknown tool: <name>"} result.

ChatResponseParser: new parseResponse() and tool-call/choice parsers;
handles both string-shaped and object-shaped tool_calls.arguments
(some upstream variants emit each shape).

Tests:
- ChatResponseTest (7 new unit tests, model-free): plain reply, tool
  calls with string arguments, object-shaped arguments, malformed
  input, ChatRequest serialization round-trip.
- LlamaModelTest: testTypedChat and testChatWithToolsLoopShortCircuits
  (model-gated).

mvn javadoc:jar BUILD SUCCESS (0 errors, 60 warnings — same as before,
none from new files).

https://claude.ai/code/session_01R4ZrEy3ptJDLuUgUKuM4Gy

* Add completeWithStats() for typed Usage/Timings/logprobs on plain completion

complete() returned only the generated text, while chat() already
exposed Usage/Timings/TokenLogprob via ChatResponse. This commit
parity-fills the plain completion path:

- New CompletionResult value type (text + Usage + Timings +
  List<TokenLogprob> + StopReason + raw JSON).
- New LlamaModel.completeWithStats(InferenceParameters) calling the
  existing non-streaming JNI path and parsing the response via a new
  CompletionResponseParser.parseCompletionResult.
- Maps the non-OAI completion fields: content -> text,
  tokens_evaluated -> Usage.promptTokens, tokens_predicted ->
  Usage.completionTokens, timings sub-object -> Timings,
  completion_probabilities -> List<TokenLogprob>, stop_type ->
  StopReason.

complete() (the String-returning overload) is unchanged for
backwards compatibility.

5 unit tests in CompletionResultTest (model-free): full response,
missing-fields defaults, stop reason mapping (eos / limit / word),
malformed input. mvn javadoc:jar BUILD SUCCESS, no new warnings.

https://claude.ai/code/session_01R4ZrEy3ptJDLuUgUKuM4Gy

* Add completeBatch / chatBatch parallel dispatch (§2.4)

Three new methods on LlamaModel that hand a list of requests to the
native scheduler at once and collect results in input order:

- completeBatch(List<InferenceParameters>)        -> List<String>
- completeBatchWithStats(List<InferenceParameters>) -> List<CompletionResult>
- chatBatch(List<ChatRequest>)                    -> List<ChatResponse>

Implementation reuses the existing CompletableFuture wrappers
(completeAsync, supplyAsync(() -> completeWithStats/chat)) and
joins them all in input order. The native worker thread runs the
upstream slot scheduler, which dispatches tasks across however many
slots ModelParameters.setParallel(N) was configured with. With the
default N=1 the batch still works correctly, just sequentially.

No JNI changes — the upstream scheduler already supports parallel
slot execution; this surfaces it as a typed Java API.

Three model-gated tests in LlamaModelTest exercise the order-preserving
contract and per-result Usage population.

mvn javadoc:jar BUILD SUCCESS, no new warnings.

https://claude.ai/code/session_01R4ZrEy3ptJDLuUgUKuM4Gy

* Add LlamaPublisher reactive-streams token publisher (§2.3 follow-up)

Backpressure-aware Publisher<LlamaOutput> on top of the existing
streaming iterator. Reactor / RxJava / Kotlin coroutines all bridge
to the Reactive Streams interface natively, so consumers wrap with
Flux.from(...) / Flowable.fromPublisher(...) / asFlow() in one line.

LlamaPublisher:
- Single-subscriber (second subscribe signals onError per RS spec).
- Each subscribe starts a dedicated emitter daemon thread.
- Demand honoured via AtomicLong + monitor: emitter blocks while
  demand == 0 and only calls iterator.next() when demand > 0.
- request(n <= 0) signals onError with IllegalArgumentException per
  reactive-streams §3.9.
- cancel() closes the underlying iterator (cooperative, same path as
  LlamaIterator.close); idempotent.
- onComplete fires on stop token, onError on any throwable from the
  iterator path.

LlamaModel:
- streamPublisher(InferenceParameters) and
  streamChatPublisher(InferenceParameters) factories.

Dependency: adds org.reactivestreams:reactive-streams 1.0.4 (~5 KB,
Java 8 compatible) to pom.xml.

Tests in LlamaPublisherTest:
- nullSubscriberThrows (model-free).
- backpressureAndCancel, singleSubscriberContract,
  invalidRequestSignalsError (model-gated).

mvn javadoc:jar BUILD SUCCESS, no new warnings.

https://claude.ai/code/session_01R4ZrEy3ptJDLuUgUKuM4Gy

* Annotate docs with shipped/open status for the §2 feature inventory

The kotlin-comparison doc and the open-issues doc were both stale
after PR #188 shipped 11 features. Bring them up to date in place
rather than introducing a separate changelog:

docs/feature-investigation-llama-stack-client-kotlin.md
- §1 capability matrix gets new rows for everything that landed
  (typed chat + tools, async wrappers, reactive Publisher, batch
  dispatch, Usage/Timings, Session, completeAsJson, TokenLogprob,
  CancellationToken, LoadProgressCallback).
- New §1.1 status legend (SHIPPED / PARTIAL / OPEN).
- Each §2.x section now starts with a Status: line summarising what
  shipped, with commit refs into this PR. §2.2/2.3/2.4/2.5/2.7/2.8/
  2.9/2.10 marked SHIPPED. §2.6 PARTIAL (locking deferred).
  §2.10 PARTIAL — cooperative cancel shipped; immediate cancel
  needs a new server-side JNI primitive (M-effort follow-up).
  §2.1 OPEN (multimodal image API).

docs/history/49be664_open_issues.md
- #113 updated STILL POSSIBLE -> FIXED in PR #188 commit 70df324,
  with a note that the richer payload (file name, bytes, weights
  flag) is intentionally not exposed because the upstream
  llama_model_params.progress_callback emits only a float.

No code changes, no test impact.

https://claude.ai/code/session_01R4ZrEy3ptJDLuUgUKuM4Gy

---------

Co-authored-by: Claude <noreply@anthropic.com>
bernardladenthin pushed a commit that referenced this pull request May 24, 2026
Three small gaps caught in a final audit of the §-doc and
the issues doc:

1. feature-investigation §1 'What java-llama.cpp already covers':
   the Session row still read 'single-threaded'. PR #189 added
   per-Session locking; relabel to 'thread-safe per instance'
   so the §1 table matches the §2.6 status block.

2. feature-investigation §4 'Suggested rollout order': all ten
   entries listed there are SHIPPED now. The section is preserved
   as a historical artifact (useful for comparing original effort
   estimates against what actually shipped, especially §2.1 which
   came in much smaller than the L estimate and §2.10 which had
   to be reverted), but a header note now states up front that it
   is no longer a roadmap.

3. docs/history/49be664_open_issues.md #113 LoadProgressCallback:
   the entry said FIXED in PR #188 and stopped there, but the
   PR #188 implementation forgot the forward declaration in
   jllama.h, which made the JNI symbol C++-mangled and produced
   an UnsatisfiedLinkError on the first call to the feature.
   PR #189 commit 36d8862 added the missing declaration and the
   feature is now actually functional (LoadProgressCallbackTest
   passes on the current CI). Add a 'Subtle issue resolved by
   PR #189' paragraph so a future reader does not think 'PR #188
   shipped this, why did it fail on the first run after merge'.

No code changes; documentation only.
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