Repository navigation
pr-set : pin #335 and #336 - #337
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bfdf28b0c2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "unslothai#157": "cudaMemcpyDefault in the ggml_cuda_cpy 2D fast path; needs a CUDA host", | ||
| "unslothai#328": "Vulkan top_k on a view off minStorageBufferOffsetAlignment (backend draft sampling over an odd vocab); covered by test-backend-ops TOP_K row cases, which need a Vulkan device, and no runner in the pipeline has one", | ||
| "unslothai#335": "backend sampling chains of inactive sequences are flagged as outputs so the graph allocator cannot alias their buffers; only observable with a block draft (DFlash / DSpark) on a multi-slot server, which needs a draft model and a GPU host the pipeline does not have", | ||
| "unslothai#336": "new architecture; its graph, converter metadata and loader are covered by test-llama-archs, which the CPU gate already runs", |
There was a problem hiding this comment.
In the CPU preflight gate, placing #336 under unchecked means its architecture is never exercised, contrary to the stated reason. .github/workflows/unsloth-pin-preflight.yml builds test-llama-archs but runs it only through feature_matrix.py, which invokes architecture probes exclusively for entries in features; entries in unchecked only produce a note. Consequently, a compiling but broken loader or graph for this newly shipped architecture can pass the gate without decoding anything. Move this entry into features with an arch check for the architecture introduced by #336 so the claimed coverage actually runs.
Useful? React with 👍 / 👎.
Pins two PRs on top of the current set:
Both PRs are based on upstream commits older than the current aged base (#335 on b11979, #336 on f5222f2, 17 commits before b11979), so pinning them does not bring newer upstream lines onto the base.
feature-checks.jsonlists both underunchecked: #335 is only observable with a draft model on a multi-slot GPU server, and #336 is covered bytest-llama-archsin the CPU gate.The merge simulation over b11979..b11982 and the CUDA verification on b11979 and b11982 are running; results will be added here.