[akapoor3518]

gemma4-validation-logs.zip
posix-validation-logs.zip
tsisim-validation-logs.zip

This PR diffs against upstream-1f368f354-baseline, a branch pointing at the exact
upstream commit (1f368f354d9edcfea9fd6a1e0989b3e7335a050f) that
consolidated-patch.patch is generated against. Since both branches share that same
upstream tree, this diff shows only the ~92 files this sync actually changes: the
Tsavorite backend (ggml/src/ggml-tsavorite/), build config (tsi-pkg-build.sh,
tsavorite-model-deployment.yaml, CMake wiring), new regression tests, and the
JIRA-2258 validation docs/logs. Companion branch jira-2258-llama-cpp-sync (PR #162)
carries the same content against origin/master directly, which is why that PR shows
~3150 changed files instead — almost all of it is upstream's own already-reviewed
commit history landing via the version bump, not new content.

Why this sync, and why now

origin/master last touched upstream at 51abc96bdc (2025-09-16). This sync's target,
1f368f354 (2026-08-13), is ~11 months of upstream ggml-org/llama.cpp history later.
That gap — not a preference for a different branch or approach — is why this lands as
one large, version-bump-style sync consolidating every Tsavorite-specific change
accumulated since 2025-09-16 into one patch file (consolidated-patch.patch), rather
than as a normal incremental PR. The stated core motivation was Gemma 4 architecture
support, which didn't exist at all on the pre-sync fork — see the Gemma 4 findings in
docs/jira-2258/gemma4-validation-logs/GEMMA4-VALIDATION-SUMMARY.md.

cubic-dev-ai review remediation (2026-08-26)

All 54 cubic-dev-ai comments across two review passes replied to and resolved
(40 in the first pass, 14 more in a second pass triggered by pushing the first round's
fixes — cubic re-reviews on every push).

• ~40 fixed directly: CMake unquoted-STREQUAL/$ENV{} bugs (the kind that break
  the whole cmake configure, not just the guarded branch), missing GGML_API on
  ggml_backend_cleanup(), an inverted log-level filter in ggml_log_internal(), a
  NaN/Inf-vs-tolerance check that silently passed bad kernel output, backend-cleanup
  ordering fixed via a BackendCleanupGuard RAII pattern (tools/cli/cli.cpp,
  tools/completion/completion.cpp, tools/server/server.cpp), the .profile
  backend callback missing its backend parameter, api_version never set for
  non-OLLAMA GGML_BACKEND_DL builds, a duplicate perf-summary print in
  common_perf_print(), a regex in model-rerun.py matching the wrong log line,
  LLAMA_OPENSSL silently defaulting OFF, ggml-tsavorite.h shipping a public header
  that depended on an uninstalled private header, plus several doc fixes (missing
  SDK_VERSION exports, a nonexistent TSI-VERSION setting, missing
  UPSTREAM_BASE_COMMIT from the review-scope doc).
• 1 false positive: a .gitmodules/ggml-tsi-kernel gitlink flagged as
  mismatched — confirmed valid via git ls-tree HEAD, no fix needed.
• 3 real bugs found while verifying the fixes above, not flagged by cubic directly:
  1. llama-cli/llama-server were never built at all under GGML_TSAVORITE=ON,
     in posix or FPGA, since this sync landed — a nesting bug in tools/CMakeLists.txt
     (the old fork built main, cli's pre-refactor name, outside the Tsavorite
     exclusion guard; this sync's port put the new cli/server subdirectories
     inside it by accident). This silently broke model-rerun.py (added in this
     same PR), which calls build-posix/bin/llama-cli. Fixed by moving ui/cli/
     server/mtmd out from under the exclusion, and fixing the FPGA .tz packaging
     step (it only knew to copy llama-completion) to include the newly-buildable
     binaries too.
  2. CMAKE_FIND_ROOT_PATH is a 4-entry CMake list (the ARM toolchain's own sysroot
     search path), not a scalar. The first attempt at a cubic-suggested rpath-link fix
     used it as one anyway, which corrupted the FPGA link
     (ld: ... Is a directory) — caught by actually rebuilding, not just trusting a
     green cmake configure. Fixed by moving the rpath-link flags to a single
     add_link_options() call in the top-level CMakeLists.txt, so it applies to
     every target via tsi-pkg-build.sh or the manual cmake -B build-fpga ...
     flow documented in the README/docs/build.md.
  3. llama-cli hung forever on any non-interactive invocation (piped/redirected
     output, no controlling TTY — exactly how model-rerun.py, added in this same PR,
     invokes it via subprocess.check_output()). cli-context.cpp's main loop, once
     the initial -p prompt is consumed, calls read_input() for the next turn and
     treats an empty returned buffer as "blank line, keep waiting" — correct for a real
     interactive TTY session, but on a non-TTY stdin getwchar() hits EOF instantly and
     forever, so the loop spins at ~100% CPU rewriting the > prompt indefinitely,
     never terminating. This is pure upstream code (cli-context.cpp isn't part of the
     Tsavorite patch) that was simply never exercised before tonight, since llama-cli
     was never buildable under Tsavorite until bug 1 above was fixed. Caught by actually
     running the freshly-built binary non-interactively, not just confirming it linked.
     Fixed by checking feof(stdin) before deciding to continue vs. break out of the
     loop — distinguishes genuine end-of-input from an interactively-submitted blank
     line, which were previously indistinguishable from the loop's point of view.
• A separate, more serious bug caught in the second review pass: an earlier fix
  had added a global set -e to tsi-pkg-build.sh to make a narrow error-propagation
  gap more reliable. cubic correctly flagged that as worse than the problem it solved —
  verified empirically: sourcing this script with set -e active and hitting any
  failing command (not just inside die()) terminates the calling shell, not just
  this script, and skips the cleanup trap entirely. Reverted; the narrower gap it was
  trying to close is left as a documented follow-up instead.
• 3 real findings deliberately left unfixed (real, but risky enough that a
  drive-by fix without dedicated regression testing would be reckless — flagged in the
  GitHub replies for a deliberate follow-up decision, not silently dropped):
  • ggml_backend_i gained new callbacks (PR @FIR-2216 - llama.cpp: expose ggml perf-summary symbols across ollama's split-binary backend boundary #158, already merged, ollama's
    split-binary boundary depends on it) without bumping GGML_BACKEND_API_VERSION
    (still 2) — a real ABI risk for any old dynamically-loaded backend .so.
  • ggml-backend.cpp's view-op scheduling forcing both a view node and its source
    onto the CPU backend is not a new bug — it's the deliberate fix from @FIR-1856 - llam.cpp:Revert Graph Split Optimization Due to Introduce… #120 (the
    revert of the graph-split optimization that crashed Qwen and hid OPU usage, @FIR-1834 - Reduce GGML Graph Splits for Tsavorite Backend — Enable L… #118).
  • struct ggml_tensor's size/layout changing under GGML_PERF/GGML_PERF_RELEASE/
    GGML_PERF_DETAIL (also from @FIR-2216 - llama.cpp: expose ggml perf-summary symbols across ollama's split-binary backend boundary #158) is safe inside this repo's own build, but a
    real hazard for external consumers of the installed public ggml.h +
    libggml.so that don't define the same flag.

Full rebuild verification after every round: posix + FPGA both clean (0 errors),
llama-cli/llama-server/llama-completion all present and packaged into
tsi-ggml-0.4.24.tz.

Posix re-validation (2026-08-26, post-remediation)

None of the fixes above touch compute kernels, but re-ran the full model set anyway
since real bugs were found and fixed along the way:

Model	Result	Duration	Generated text
tinyllama-vo-5m-para.gguf	✅ 0 warnings	5s	My cat's name is Tim. He
Gemma3-270M-F32.gguf	✅ 0 warnings	33s	My cat's name is "Cat"
Tiny-Llama-v0.3-FP32-1.1B-F32.gguf	✅ 0 warnings	3m55s	My cat's name is Luna.
Llama3.2:1B-1.2B-F32.gguf	✅ 0 warnings	3m59s	My cat's name is "Bella"
qwen2-0_5b-instruct-q5_k_m.gguf	✅ 0 warnings	1m30s	My cat's name is Muffy. She
Qwen2.5-0.5B-Q4_K_M.gguf	✅ 0 warnings	1m25s	My cat's name is Zane. He
Gemma4-e2b-5.1B-Q4_K_M.gguf	⚠️ known upstream bug, not a regression -- fails to load (wrong number of tensors), independently confirmed against ggml-org/llama.cpp#27349	4s	(fails to load)
Gemma4-12b-Q4_K_M.gguf	✅ 0 warnings	29m18s	My cat's name is **"Luna0 (matches the already-documented Tsavorite-vs-CPU formatting divergence, not corruption)

7/8 models pass cleanly with zero warnings; the 8th (Gemma4-e2b) fails identically to before this PR's changes, a pre-existing upstream loading bug unrelated to the Tsavorite backend (see the Gemma 4 investigation this PR already contains for detail).

Raw logs gzipped and attached to this PR description rather than committed to the
repo tree. llama-cli smoke-tested end to end for the first time (it was never
buildable under Tsavorite before this fix): clean run, user_dram_size_gb=16 correctly read, generated "I'm, exited cleanly in 6s (previously spun forever on any non-interactive invocation -- see the cli-context.cpp fix above).

Review this PR's file-by-file diff for the actual code review of the ~92 files that
matter. Once approved here, the same content lands via #162.

---

Summary by cubic

Review-only branch for JIRA-2258 that diffs against the exact upstream baseline consolidated-patch.patch is generated from, exposing only the ~93 files this sync actually changes instead of the ~3150-file diff against master. Do not merge this branch; the real merge is jira-2258-llama-cpp-sync.

What changed

• Carries the full Tsavorite delta as consolidated-patch.patch, with UPSTREAM_BASE_COMMIT pinning its regeneration base.
• Adds the backend (ggml/src/ggml-tsavorite/), build tooling (tsi-pkg-build.sh, tsavorite-model-deployment.yaml, CMake wiring), repro tools under examples/simple/, and a model-rerun.py validation harness.
• Fixes llama-cli and llama-server building under GGML_TSAVORITE (they previously never built).
• Makes FPGA rpath-link a single global add_link_options() call and routes TOOLBOX_DIR overrides (normalized to an absolute path) through one TSAVORITE_TOOLBOX_ROOT for both include and link roots.
• Removes set -e from tsi-pkg-build.sh; sourcing it could kill the caller's shell.
• Fixes llama-cli's 100% CPU spin on piped, non-TTY input by tracking EOF separately from blank lines via a bool* eof out-param; the loop breaks on genuine EOF (getline failure/Ctrl+D/WEOF), and the earlier isatty() heuristic that exited early on embedded blank lines is removed.
• Makes ggml_backend_cleanup() explicit (void) to fix an upstream -Werror=strict-prototypes CI failure.
• Fixes validation findings: non-128-byte-aligned broadcast chunking, 2D scalar broadcast, and unaligned-memory warnings.
• Adds GGML_PERF-guarded op profiling with a dedicated Tsavorite summary, a GGML_LOG_LEVEL_TSAVORITE log level, and backend profile/cleanup hooks.
• Skips upstream examples and tests that don't build on Tsavorite via GGML_TSAVORITE CMake guards.
• Raises the user_dram_size_gb deployment default from 8 to 16.
• Keeps posix, gemma4, and tsisim validation summaries under docs/jira-2258/: 4/4 posix models are byte-identical old vs new and Gemma 4 12b loads; raw per-run logs are attached to this PR instead of committed.

^{Written for commit 1b3aaa1. Summary will update on new commits.}

[cubic-dev-ai]

8 issues found and verified against the latest diff

Not reviewed (too large): consolidated-patch.patch (~12,637 lines), ggml/src/ggml-tsavorite/ggml-tsavorite.cpp (~8,221 lines) - if these are generated or fixture files, add them to ignored paths to exclude them from future reviews.

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="ggml/src/ggml-backend-impl.h">

<violation number="1" location="ggml/src/ggml-backend-impl.h:142">
P1: When an older dynamically loaded backend reports API version 2, the new host interprets its old `device` and `context` fields as these callbacks. Bump `GGML_BACKEND_API_VERSION` or preserve a compatible `ggml_backend` layout when extending `ggml_backend_i`.</violation>
</file>

<file name="ggml/CMakeLists.txt">

<violation number="1" location="ggml/CMakeLists.txt:334">
P2: After installation, any consumer including `<ggml-tsavorite.h>` fails because this entry ships the header without its private `TestModel.h` and SDK-only `HostShimCAPI.h` dependencies. Make the public header self-contained or package and expose those dependencies before adding it to `GGML_PUBLIC_HEADERS`.</violation>
</file>

<file name="tools/cli/cli.cpp">

<violation number="1" location="tools/cli/cli.cpp:31">
P1: In perf builds, this callback suppresses all ordinary llama warnings and errors because it forwards only `GGML_LOG_LEVEL_TSAVORITE`. Forward `GGML_LOG_LEVEL_WARN` and `GGML_LOG_LEVEL_ERROR` as well, while retaining the Tsavorite level if its diagnostics are required.</violation>
</file>

<file name="ggml/src/ggml-backend-reg.cpp">

<violation number="1" location="ggml/src/ggml-backend-reg.cpp:586">
P1: In non-OLLAMA `GGML_BACKEND_DL` builds, this load attempt always rejects the Tsavorite module for an API-version mismatch. Set `g_ggml_backend_tsavorite_reg.api_version` for every dynamic-build path before loading it.</violation>
</file>

<file name="ggml/src/ggml-backend.cpp">

<violation number="1" location="ggml/src/ggml-backend.cpp:1141">
P1: When a graph contains a view, this block overwrites both the view and its source with the CPU backend. Preserve the source's assigned backend; views should follow it rather than forcing accelerator results onto CPU.</violation>

<violation number="2" location="ggml/src/ggml-backend.cpp:2457">
P1: When a live Tsavorite backend exists, `ggml_backend_cleanup()` finalizes its runtime before the backend is freed. Defer this call until all Tsavorite backends are destroyed, or make teardown ownership explicit, because later `tsi_dealloc` and device release use finalized state.</violation>
</file>

<file name="ggml/include/ggml.h">

<violation number="1" location="ggml/include/ggml.h:714">
P1: When the library and consumer use different perf flags, `struct ggml_tensor` has different size and layout. Keep this public struct layout identical in every build by moving counters out-of-line or exposing the same tail unconditionally, and ensure the build definition reaches all consumers.</violation>
</file>

<file name=".gitmodules">

<violation number="1" location=".gitmodules:2">
P1: In an already-patched checkout, this metadata has no matching `ggml-tsi-kernel` gitlink, so `git submodule update --init --recursive` does nothing and the TSI build fails when `ensure_submodules()` finds the directory missing. Add the pinned `ggml-tsi-kernel` gitlink, or register it before `apply_consolidated_patch()` returns for already-patched trees.</violation>
</file>

<sub>Tip: instead of fixing issues one by one fix them all with cubic

Re-trigger cubic</sub>

[cubic-dev-ai]

6 issues found across 31 files (changes from recent commits).

Not reviewed (too large): consolidated-patch.patch (~519 lines) - if these are generated or fixture files, add them to ignored paths to exclude them from future reviews.

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="tools/completion/CMakeLists.txt">

<violation number="1" location="tools/completion/CMakeLists.txt:21">
P1: Direct FPGA builds documented in `README.md` and `docs/build.md` do not receive the global linker flags described here. After removing the target-specific flags, `llama-completion` can fail to link with unresolved transitively-needed `tsi_*` symbols; retain a CMake-side fallback or update the direct-build path to provide these library paths.</violation>
</file>

<file name="ggml/src/ggml-backend-impl.h">

<violation number="1" location="ggml/src/ggml-backend-impl.h:148">
P2: When an older dynamically loaded backend is used, the loader still accepts API version 2 even though `profile` now has a different function type. Bump the backend API version or add an explicit compatibility path for the old callback signature.</violation>
</file>

<file name="common/common.h">

<violation number="1" location="common/common.h:468">
P1: When `GGML_BACKEND_DL=ON`, neither macro reaches `common.h`: CMake skips `GGML_USE_TSAVORITE`, and `GGML_TSAVORITE` is not a compiler definition. `fit_params` therefore remains true, so `common_init_result` can still run the Tsavorite-crashing probe; propagate a Tsavorite definition to `llama-common` for dynamic builds.</violation>
</file>

<file name=".github/copilot-instructions.md">

<violation number="1" location=".github/copilot-instructions.md:16">
P3: This added line violates the repository's ASCII-only documentation rule. Replace the em dash with an ASCII hyphen.</violation>
</file>

<file name="common/CMakeLists.txt">

<violation number="1" location="common/CMakeLists.txt:180">
P1: When FPGA is configured with the documented direct CMake command instead of `tsi-pkg-build.sh`, this branch provides no `-Wl,-rpath-link` for the Tsavorite runtime libraries, so final executable links fail on transitive `tsi_*` dependencies. Expose the required link options through the CMake target/interface, or update the direct-build instructions to pass equivalent flags.</violation>
</file>

<file name="tsi-pkg-build.sh">

<violation number="1" location="tsi-pkg-build.sh:286">
P1: When a sourced build hits a handled failure, this global `set -e` exits the caller shell instead of returning the error as `die()` promises, and skips the `RETURN` cleanup that restores shell options, cwd-related state, and the prior virtual environment. Do not enable errexit in the caller; propagate sourced failures with explicit checks or execute the build in a controlled subshell.</violation>
</file>

<sub>Tip: instead of fixing issues one by one fix them all with cubic
Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic</sub>

[cubic-dev-ai]

P1: Direct FPGA builds documented in README.md and docs/build.md do not receive the global linker flags described here. After removing the target-specific flags, llama-completion can fail to link with unresolved transitively-needed tsi_* symbols; retain a CMake-side fallback or update the direct-build path to provide these library paths.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At tools/completion/CMakeLists.txt, line 21:

<comment>Direct FPGA builds documented in `README.md` and `docs/build.md` do not receive the global linker flags described here. After removing the target-specific flags, `llama-completion` can fail to link with unresolved transitively-needed `tsi_*` symbols; retain a CMake-side fallback or update the direct-build path to provide these library paths.</comment>

<file context>
@@ -18,13 +18,9 @@ set(TARGET llama-completion)
-	target_link_directories(${TARGET} PRIVATE ${SYSROOT_PATH_FOR_LIBRARIES})
-endif()
-endif()
+# rpath-link for transitively-needed tsi_* symbols on FPGA builds is handled
+# globally via -DCMAKE_EXE_LINKER_FLAGS in tsi-pkg-build.sh (FPGA_TOOLBOX_LIB_DIR /
+# FPGA_RUNTIME_LIB_DIR) -- no per-target linker flags needed here.
</file context>

[akapoor3518]

Good catch - removing that block also removed the only rpath-link source for anyone using the direct/manual cmake flow from README/docs/build.md instead of tsi-pkg-build.sh. Moved the fix up to the top-level CMakeLists.txt instead, using add_link_options() once for the whole fpga build so it applies to every target (llama-cli, llama-completion, llama-server, simple-backend-tsi) regardless of whether tsi-pkg-build.sh or the manual cmake commands are used. Verified with a clean FPGA rebuild.

[cubic-dev-ai]

P1: When GGML_BACKEND_DL=ON, neither macro reaches common.h: CMake skips GGML_USE_TSAVORITE, and GGML_TSAVORITE is not a compiler definition. fit_params therefore remains true, so common_init_result can still run the Tsavorite-crashing probe; propagate a Tsavorite definition to llama-common for dynamic builds.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At common/common.h, line 468:

<comment>When `GGML_BACKEND_DL=ON`, neither macro reaches `common.h`: CMake skips `GGML_USE_TSAVORITE`, and `GGML_TSAVORITE` is not a compiler definition. `fit_params` therefore remains true, so `common_init_result` can still run the Tsavorite-crashing probe; propagate a Tsavorite definition to `llama-common` for dynamic builds.</comment>

<file context>
@@ -465,13 +465,20 @@ struct common_params {
     int32_t main_gpu           = 0;     // the GPU that is used for scratch and small tensors
     float   tensor_split[128]  = {0};   // how split tensors should be distributed across GPUs
-#ifdef GGML_TSAVORITE
+#if defined(GGML_TSAVORITE) || defined(GGML_USE_TSAVORITE)
     // fit_params (a new upstream feature) constructs a throwaway llama_context per
     // device purely to probe its free memory, which initializes that device's ggml
</file context>

[akapoor3518]

This one I'm not fixing as a #ifdef tweak - it can't actually be fixed that way. GGML_BACKEND_DL is a compile-time flag; whether Tsavorite ends up dynamically loaded is a runtime decision, so no header-level #ifdef can know that at the point common.h is compiled. Closing this properly needs a runtime check (something like "if any loaded backend is tsavorite, disable fit_params"), not a default tweak here. Worth noting: this codebase's own build tooling (tsi-pkg-build.sh) never sets GGML_BACKEND_DL=ON for either posix or fpga, so this specific gap isn't live in any build this repo's scripts actually produce today. Flagging as a real but narrow follow-up.

[cubic-dev-ai]

P1: When FPGA is configured with the documented direct CMake command instead of tsi-pkg-build.sh, this branch provides no -Wl,-rpath-link for the Tsavorite runtime libraries, so final executable links fail on transitive tsi_* dependencies. Expose the required link options through the CMake target/interface, or update the direct-build instructions to pass equivalent flags.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At common/CMakeLists.txt, line 180:

<comment>When FPGA is configured with the documented direct CMake command instead of `tsi-pkg-build.sh`, this branch provides no `-Wl,-rpath-link` for the Tsavorite runtime libraries, so final executable links fail on transitive `tsi_*` dependencies. Expose the required link options through the CMake target/interface, or update the direct-build instructions to pass equivalent flags.</comment>

<file context>
@@ -176,11 +176,14 @@ if (LLAMA_LLGUIDANCE)
 if (GGML_TSAVORITE)
-    if (${GGML_TSAVORITE_TARGET} STREQUAL fpga)
+    if ("${GGML_TSAVORITE_TARGET}" STREQUAL "fpga")
+        # rpath-link for transitively-needed tsi_* symbols is handled globally via
+        # -DCMAKE_EXE_LINKER_FLAGS in tsi-pkg-build.sh (FPGA_TOOLBOX_LIB_DIR /
+        # FPGA_RUNTIME_LIB_DIR) -- no per-target linker flags needed here. A prior
</file context>

[akapoor3518]

Same root cause/fix as the tools/cli and tools/completion comments - moved this to a single add_link_options() call in the top-level CMakeLists.txt instead of duplicating (and, before, getting wrong) the same rpath-link logic per-target. Covers the manual-cmake-build gap too now.

[cubic-dev-ai]

P1: When a sourced build hits a handled failure, this global set -e exits the caller shell instead of returning the error as die() promises, and skips the RETURN cleanup that restores shell options, cwd-related state, and the prior virtual environment. Do not enable errexit in the caller; propagate sourced failures with explicit checks or execute the build in a controlled subshell.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At tsi-pkg-build.sh, line 286:

<comment>When a sourced build hits a handled failure, this global `set -e` exits the caller shell instead of returning the error as `die()` promises, and skips the `RETURN` cleanup that restores shell options, cwd-related state, and the prior virtual environment. Do not enable errexit in the caller; propagate sourced failures with explicit checks or execute the build in a controlled subshell.</comment>

<file context>
@@ -277,6 +277,13 @@ export SDK_VERSION
+# so this correctly aborts the whole call chain instead of silently
+# continuing with invalid state. cleanup()'s trap restores this (and every
+# other shell option) via __TSI_OLD_SET on every return/exit path below.
+set -e
 __TSI_SCRIPT_PATH="${BASH_SOURCE[0]:-$0}"
 __TSI_SCRIPT_DIR="$(cd "$(dirname "${__TSI_SCRIPT_PATH}")" 2>/dev/null && pwd)"
</file context>

[akapoor3518]

You're right and this is a real problem - verified it empirically. Sourcing this script with set -e active and hitting ANY failing command (not just inside die()) kills the calling shell outright, not just this script, and skips cleanup() entirely since only a RETURN trap is registered for the sourced case, which doesn't fire on an errexit-triggered process exit. Reverted the set -e. The narrower thing it was trying to fix (cmd || die "msg" not aborting the caller when die() returns 1) is still open, but it's a much smaller problem than "sourcing this can kill your terminal" - noted as a known follow-up in a comment at that spot instead.

[cubic-dev-ai]

P2: When an older dynamically loaded backend is used, the loader still accepts API version 2 even though profile now has a different function type. Bump the backend API version or add an explicit compatibility path for the old callback signature.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At ggml/src/ggml-backend-impl.h, line 148:

<comment>When an older dynamically loaded backend is used, the loader still accepts API version 2 even though `profile` now has a different function type. Bump the backend API version or add an explicit compatibility path for the old callback signature.</comment>

<file context>
@@ -145,7 +145,7 @@ extern "C" {
         void                      (*reset)              (ggml_backend_t backend);
         // (optional) print backend-specific profiling info
-        void                      (*profile)            ();
+        void                      (*profile)            (ggml_backend_t backend);
     };
 
</file context>

[akapoor3518]

Same ABI-version gap as the other comment on this line - agreed it's real, not fixing it tonight. graph_reserve/buffer_size/reset/profile were all added to ggml_backend_i without bumping GGML_BACKEND_API_VERSION (still 2), in an already-merged PR (#158) that ollama's split-binary boundary depends on. Bumping the version now is a real decision about compatibility with whatever's already built against version 2 on the ollama side - not something to do as a side effect of this PR.

[cubic-dev-ai]

P3: This added line violates the repository's ASCII-only documentation rule. Replace the em dash with an ASCII hyphen.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At .github/copilot-instructions.md, line 16:

<comment>This added line violates the repository's ASCII-only documentation rule. Replace the em dash with an ASCII hyphen.</comment>

<file context>
@@ -13,6 +13,10 @@ history landing via the version bump — **do not review it file-by-file**.
 For this PR type, scope review to only:
 - `consolidated-patch.patch` at the repo root — every Tsavorite-specific change, as one
   patch file. This is the actual diff to read.
+- `UPSTREAM_BASE_COMMIT` at the repo root — the upstream SHA this sync vendored forward
+  to and that `consolidated-patch.patch` was regenerated against. Check it matches the
+  commit named in the PR title/description; a mismatch here means the patch may not
</file context>

Suggested change

	- `UPSTREAM_BASE_COMMIT` at the repo root — the upstream SHA this sync vendored forward
	- `UPSTREAM_BASE_COMMIT` at the repo root - the upstream SHA this sync vendored forward

[akapoor3518]

Leaving this one - the rest of this same file already uses em dashes throughout (lines 11, 14, 20, 25, 33, all pre-existing), so changing only my added line would make it inconsistent with the file's own established style rather than fixing anything. If there's an actual desire to convert the whole file to ASCII hyphens that's a one-line sed across the file, happy to do that as its own pass, but not singling out my line.

[cubic-dev-ai]

1 issue found across 6 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="consolidated-patch.patch">

<violation number="1" location="consolidated-patch.patch:12456">
P1: When the user sends Ctrl-D on an empty prompt, `readline_advanced()` returns an empty buffer without setting `feof(stdin)`, so this condition is false and `cli_context::run()` continues prompting indefinitely. Propagate the readline end-of-stream result through `read_input()` instead of relying only on the C stream EOF flag.</violation>
</file>

<sub>Tip: instead of fixing issues one by one fix them all with cubic
Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic</sub>

[cubic-dev-ai]

P1: When the user sends Ctrl-D on an empty prompt, readline_advanced() returns an empty buffer without setting feof(stdin), so this condition is false and cli_context::run() continues prompting indefinitely. Propagate the readline end-of-stream result through read_input() instead of relying only on the C stream EOF flag.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At consolidated-patch.patch, line 12456:

<comment>When the user sends Ctrl-D on an empty prompt, `readline_advanced()` returns an empty buffer without setting `feof(stdin)`, so this condition is false and `cli_context::run()` continues prompting indefinitely. Propagate the readline end-of-stream result through `read_input()` instead of relying only on the C stream EOF flag.</comment>

<file context>
@@ -12404,6 +12426,39 @@ index 8449cdbaf..362cda14b 100644
++        // through llama-cli, previously untested since llama-cli was never
++        // buildable under GGML_TSAVORITE before this sync.
+         if (buffer.empty()) {
++            if (std::feof(stdin)) {
++                break;
++            }
</file context>

[akapoor3518]

Same fix as the cli-context.cpp comment - see that reply. Patch regenerated to reflect it.

[cubic-dev-ai]

All reported issues were addressed across 5 files (changes from recent commits).

<sub>Tip: Review your code locally with the cubic CLI to iterate faster.

Fix all with cubic | Re-trigger cubic</sub>

[cubic-dev-ai]

1 issue found across 1 file (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="CMakeLists.txt">

<violation number="1" location="CMakeLists.txt:87">
P2: Fresh upstream bootstraps do not receive this fix: `apply_consolidated_patch()` applies `consolidated-patch.patch`, whose `CMakeLists.txt` hunk still uses the old link-path logic and default include path. Regenerate `consolidated-patch.patch` so custom `TOOLBOX_DIR` builds reproduce this change.</violation>
</file>

<sub>Tip: Review your code locally with the cubic CLI to iterate faster.

Fix all with cubic | Re-trigger cubic</sub>

[cubic-dev-ai]

P2: Fresh upstream bootstraps do not receive this fix: apply_consolidated_patch() applies consolidated-patch.patch, whose CMakeLists.txt hunk still uses the old link-path logic and default include path. Regenerate consolidated-patch.patch so custom TOOLBOX_DIR builds reproduce this change.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At CMakeLists.txt, line 87:

<comment>Fresh upstream bootstraps do not receive this fix: `apply_consolidated_patch()` applies `consolidated-patch.patch`, whose `CMakeLists.txt` hunk still uses the old link-path logic and default include path. Regenerate `consolidated-patch.patch` so custom `TOOLBOX_DIR` builds reproduce this change.</comment>

<file context>
@@ -79,6 +79,18 @@ if (GGML_TSAVORITE)
+    # point at the same toolbox install (same TOOLBOX_DIR override, if any) --
+    # otherwise a non-default TOOLBOX_DIR would link against one toolbox build
+    # while compiling against another's headers.
+    if (DEFINED TOOLBOX_DIR AND NOT "${TOOLBOX_DIR}" STREQUAL "")
+        set(TSAVORITE_TOOLBOX_ROOT "${TOOLBOX_DIR}")
+    elseif (DEFINED ENV{TOOLBOX_DIR} AND NOT "$ENV{TOOLBOX_DIR}" STREQUAL "")
</file context>

[akapoor3518]

This one is stale - the review run here caught the window between the CMakeLists.txt fix and the regenerate-patch commit that followed it. consolidated-patch.patch already has TSAVORITE_TOOLBOX_ROOT in the current HEAD (confirmed via grep), so a fresh upstream bootstrap does get this fix. I regenerate the patch as a matching follow-up commit after every CMakeLists.txt change in this PR for exactly this reason.

[cubic-dev-ai]

1 issue found across 4 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="common/console.cpp">

<violation number="1" location="common/console.cpp:1051">
P2: When simple I/O reads a final line without a trailing newline, `std::getline` has already reached EOF but this reset leaves `eof` false. Propagate `std::cin.eof()` or `std::wcin.eof()` after a successful read so piped input does not trigger an extra prompt and read.</violation>
</file>

<sub>Tip: Review your code locally with the cubic CLI to iterate faster.

Fix all with cubic | Re-trigger cubic</sub>

[cubic-dev-ai]

P2: When simple I/O reads a final line without a trailing newline, std::getline has already reached EOF but this reset leaves eof false. Propagate std::cin.eof() or std::wcin.eof() after a successful read so piped input does not trigger an extra prompt and read.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At common/console.cpp, line 1051:

<comment>When simple I/O reads a final line without a trailing newline, `std::getline` has already reached EOF but this reset leaves `eof` false. Propagate `std::cin.eof()` or `std::wcin.eof()` after a successful read so piped input does not trigger an extra prompt and read.</comment>

<file context>
@@ -1040,15 +1040,24 @@ namespace console {
-    static bool readline_simple(std::string & line, bool multiline_input) {
+    static bool readline_simple(std::string & line, bool multiline_input, bool * eof) {
+        if (eof) {
+            *eof = false;
+        }
 #if defined(_WIN32)
</file context>

[akapoor3518]

Checked this against actual behavior - functionally it is not a bug: on the next call, getline() fails against the already-exhausted stream, eof correctly comes back true, and the loop breaks and exits cleanly (verified: piping a final line with no trailing newline into llama-cli still processes the line and exits 0, no hang, no spin).

What you are pointing at is real but cosmetic: one extra bare prompt line gets printed before the loop discovers EOF and exits. That is not unique to the missing-trailing-newline case though - it happens identically for input that DOES end with a newline too, since you cannot know a stream has ended until you attempt a read and get the failure back. Fixing it fully would mean restructuring read_input() to defer printing the prompt until a read has already succeeded, which crosses the layer boundary between console.cpp's return value and cli-context.cpp's prompt logic - a real change, but for a cosmetic artifact rather than a correctness issue, so I am leaving it as-is rather than expanding scope on this pass.