copilot-pull-request-reviewer[bot]
(Migrated from github.com)
reviewed 2025-07-12 20:44:36 +00:00
copilot-pull-request-reviewer[bot]
(Migrated from github.com)
left a comment
Copy Link
Copy Source
Pull Request Overview
This PR adds a proof-of-concept CLI application (stream_app) that integrates llama.cpp to generate intent-based suggestions in a conversational loop.
Introduces main.c which loads a Phi model via llama.cpp, tokenizes a prompt, and streams generated tokens.
Adds build configuration (Makefile, submodule, README) to fetch and compile llama.cpp dependencies.
Provides a README with setup and usage instructions.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.
Show a summary per file
File
Description
stream/thirdparty/llama.cpp
Adds llama.cpp as a git submodule for local inference
stream/main.c
Implements prompt assembly, tokenization, model inference, and streaming output
stream/README.md
Documents PoC purpose, dependencies, build and run instructions
stream/Makefile
Defines build rules for stream_app, including include paths and libraries
stream/.gitignore
Ignores build artifacts, cache, and model weight files
.gitmodules
Registers llama.cpp submodule
## Pull Request Overview
This PR adds a proof-of-concept CLI application (`stream_app`) that integrates llama.cpp to generate intent-based suggestions in a conversational loop.
- Introduces `main.c` which loads a Phi model via llama.cpp, tokenizes a prompt, and streams generated tokens.
- Adds build configuration (Makefile, submodule, README) to fetch and compile llama.cpp dependencies.
- Provides a README with setup and usage instructions.
### Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.
<details>
<summary>Show a summary per file</summary>
| File | Description |
| ------------------------- | ----------------------------------------------------------------- |
| stream/thirdparty/llama.cpp | Adds llama.cpp as a git submodule for local inference |
| stream/main.c | Implements prompt assembly, tokenization, model inference, and streaming output |
| stream/README.md | Documents PoC purpose, dependencies, build and run instructions |
| stream/Makefile | Defines build rules for `stream_app`, including include paths and libraries |
| stream/.gitignore | Ignores build artifacts, cache, and model weight files |
| .gitmodules | Registers llama.cpp submodule |
</details>
[nitpick] The include path is overly verbose and ties the code to repository layout. Since your Makefile adds -Ithirdparty/llama.cpp/ggml/include, using #include <ggml-backend.h> improves readability.
#include <ggml-backend.h>
[nitpick] The include path is overly verbose and ties the code to repository layout. Since your Makefile adds `-Ithirdparty/llama.cpp/ggml/include`, using `#include <ggml-backend.h>` improves readability.
```suggestion
#include <ggml-backend.h>
```
snprintf(prompt,sizeof(prompt),"<|system|>You are a helpful assistant. Your goal is to take in what the user is doing and return 3 predictive actions/3 suggestions based on what the user is trying to do: e.g. change page title when in google sheets, or calculate sum, create chart.<|end|>\n<|user|>%s<|end|>\n<|assistant|>",user_input);
[nitpick] The hard-coded token prediction limit (1000) should be extracted into a constant or exposed as a command-line option for easier tuning.
// Default number of tokens to predict
const int DEFAULT_N_PREDICT = 1000;
int n_predict = DEFAULT_N_PREDICT;
// Check for command-line argument to override token prediction limit
if (argc > 1) {
n_predict = atoi(argv[1]);
if (n_predict <= 0) {
fprintf(stderr, "Invalid token prediction limit. Using default: %d\n", DEFAULT_N_PREDICT);
n_predict = DEFAULT_N_PREDICT;
}
}
// load dynamic backends
[nitpick] The hard-coded token prediction limit (`1000`) should be extracted into a constant or exposed as a command-line option for easier tuning.
```suggestion
// Default number of tokens to predict
const int DEFAULT_N_PREDICT = 1000;
int n_predict = DEFAULT_N_PREDICT;
// Check for command-line argument to override token prediction limit
if (argc > 1) {
n_predict = atoi(argv[1]);
if (n_predict <= 0) {
fprintf(stderr, "Invalid token prediction limit. Using default: %d\n", DEFAULT_N_PREDICT);
n_predict = DEFAULT_N_PREDICT;
}
}
// load dynamic backends
```
The leading '-' negates the return value of llama_tokenize, resulting in a negative token count. Remove the '-' to correctly capture the number of tokens.
The leading '-' negates the return value of `llama_tokenize`, resulting in a negative token count. Remove the '-' to correctly capture the number of tokens.
```suggestion
llama_tokenize(vocab, prompt, strlen(prompt), NULL, 0, true, true);
```
You are not authorized to merge this pull request.
This pull request can be merged automatically.
View command line instructions
Checkout
From your project repository, check out a new branch and test the changes.
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Pull Request Overview
This PR adds a proof-of-concept CLI application (
stream_app) that integrates llama.cpp to generate intent-based suggestions in a conversational loop.main.cwhich loads a Phi model via llama.cpp, tokenizes a prompt, and streams generated tokens.Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.
Show a summary per file
stream_app, including include paths and libraries@@ -0,0 +4,4 @@## Todo- [x] Integrate llama.cpp with local inference. This will set us up for building many parts of experience.- [ ] Disect what llama is doing and what the Phi model is doing.Typo in the list: 'Disect' should be spelled 'Dissect'.
@@ -0,0 +1,176 @@#include "thirdparty/llama.cpp/ggml/include/ggml-backend.h"[nitpick] The include path is overly verbose and ties the code to repository layout. Since your Makefile adds
-Ithirdparty/llama.cpp/ggml/include, using#include <ggml-backend.h>improves readability.@@ -0,0 +16,4 @@int main(int argc, char *argv[]) {printf("Hello world\n");[nitpick] Remove this leftover debugging print; the generic greeting is unnecessary in the production proof-of-concept.
@@ -0,0 +25,4 @@snprintf(prompt, sizeof(prompt), "<|system|>You are a helpful assistant. Your goal is to take in what the user is doing and return 3 predictive actions/3 suggestions based on what the user is trying to do: e.g. change page title when in google sheets, or calculate sum, create chart.<|end|>\n<|user|>%s<|end|>\n<|assistant|>", user_input);// number of layers to offload to the GPUint ngl = 99;[nitpick] Using a magic number for GPU layer count reduces clarity. Consider defining a named constant or making this a runtime parameter.
@@ -0,0 +30,4 @@int n_predict = 1000;// load dynamic backends[nitpick] The hard-coded token prediction limit (
1000) should be extracted into a constant or exposed as a command-line option for easier tuning.@@ -0,0 +51,4 @@// find the number of tokens in the promptconst int n_prompt =-llama_tokenize(vocab, prompt, strlen(prompt), NULL, 0, true, true);The leading '-' negates the return value of
llama_tokenize, resulting in a negative token count. Remove the '-' to correctly capture the number of tokens.View command line instructions
Checkout
From your project repository, check out a new branch and test the changes.