indices out of range fixed - #16
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideClamp span indices to the valid sequence length before gathering, preventing out-of-range index errors during span extraction. Flow diagram for clamped span extraction logicflowchart TD
A[Start extract_elements] --> B[Input sequence tensor of shape B x L x D]
B --> C[Input indices tensor of shape B x K]
C --> D[Compute B L D from sequence.size]
D --> E[Clamp indices to range 0 to L - 1 using torch.clamp]
E --> F[Unsqueeze indices to shape B x K x 1]
F --> G[Expand indices along D to shape B x K x D]
G --> H[Use torch.gather on sequence dim 1 with expanded_indices]
H --> I[Output extracted_elements tensor of shape B x K x D]
I --> J[End extract_elements]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Summary of ChangesHello @arthrod, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves a potential issue where tensor indices could exceed the valid range of a sequence during element extraction. By introducing a clamping operation, it ensures that all indices used for gathering elements are within the acceptable bounds, thereby preventing out-of-range errors and making the Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
📝 WalkthroughWalkthroughThis PR expands GLiNER with Ray Serve deployment, model loading options, precomputed prompts, relation extraction, batched decoding, tensorized preprocessing, benchmark tooling, CI/release workflows, training updates, and substantial documentation and test coverage. ChangesCore inference and relation extraction
Ray Serve deployment
Tooling and project operations
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The new
B, L, D = sequence.size()unpacking assumes a 3D tensor and will break ifextract_elementsis ever called with a different rank; consider asserting the expected shape or usingsequence.size(-1)andsequence.size(1)to be more defensive. - Clamping indices with
torch.clamp(indices, 0, L - 1)silently masks out-of-range bugs by snapping to the boundary; consider validating and raising an error (or at least logging) when indices are out of range instead of coercing them.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `B, L, D = sequence.size()` unpacking assumes a 3D tensor and will break if `extract_elements` is ever called with a different rank; consider asserting the expected shape or using `sequence.size(-1)` and `sequence.size(1)` to be more defensive.
- Clamping indices with `torch.clamp(indices, 0, L - 1)` silently masks out-of-range bugs by snapping to the boundary; consider validating and raising an error (or at least logging) when indices are out of range instead of coercing them.
## Individual Comments
### Comment 1
<location> `gliner/modeling/span_rep.py:377-378` </location>
<code_context>
- D = sequence.size(-1)
-
- # Expand indices to [B, K, D]
+ B, L, D = sequence.size()
+ indices = torch.clamp(indices, 0, L - 1)
expanded_indices = indices.unsqueeze(2).expand(-1, -1, D)
-
</code_context>
<issue_to_address>
**question (bug_risk):** Clamping out-of-range indices changes failure mode from explicit error to silent boundary clipping.
`torch.gather` used to raise on out-of-bounds indices, clearly exposing invalid inputs. With `torch.clamp`, those invalid indices now silently map to boundary positions, which can hide upstream bugs. If you need robustness, either (a) validate indices and fail fast (e.g., assertion) or (b) make this clamping behavior explicit/optional so callers understand the changed semantics.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| B, L, D = sequence.size() | ||
| indices = torch.clamp(indices, 0, L - 1) |
There was a problem hiding this comment.
question (bug_risk): Clamping out-of-range indices changes failure mode from explicit error to silent boundary clipping.
torch.gather used to raise on out-of-bounds indices, clearly exposing invalid inputs. With torch.clamp, those invalid indices now silently map to boundary positions, which can hide upstream bugs. If you need robustness, either (a) validate indices and fail fast (e.g., assertion) or (b) make this clamping behavior explicit/optional so callers understand the changed semantics.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request correctly addresses a potential index-out-of-range error in the extract_elements function by clamping indices to the valid sequence length. This is a robust fix that prevents runtime errors and improves the overall stability of the model. The implementation is clean and effective. Great work on this important bug fix.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
gliner/modeling/span_rep.py (1)
377-378: Silent clamping may mask upstream bugs.Clamping out-of-range indices prevents
IndexError, but also silently produces potentially incorrect results when indices are genuinely invalid. Consider adding a debug-mode warning or assertion to help catch upstream issues during development:💡 Optional: Add debug logging for out-of-bounds indices
def extract_elements(sequence, indices): B, L, D = sequence.size() + if torch.any(indices < 0) or torch.any(indices >= L): + # Consider logging or raising in debug mode + pass # or: logging.debug(f"Clamping {((indices < 0) | (indices >= L)).sum()} out-of-bounds indices") indices = torch.clamp(indices, 0, L - 1)Also, static analysis indicates
Bis unused—prefix with underscore to clarify intent:- B, L, D = sequence.size() + _B, L, D = sequence.size()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@gliner/modeling/span_rep.py` around lines 377 - 378, Replace the silent clamping of indices in span_rep logic: change the unused variable name B to _B (e.g., "_B, L, D = sequence.size()") and add a debug-mode check before calling torch.clamp that detects out-of-range indices (e.g., check torch.any(indices < 0 or indices >= L) or equivalent) and either assert or emit a warning/log via the module logger so upstream bugs aren’t silently masked; keep the clamp as a fallback for production but ensure the debug assertion/warning references the symbols sequence, indices, L and torch.clamp so it’s easy to locate.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@gliner/modeling/span_rep.py`:
- Around line 377-378: Replace the silent clamping of indices in span_rep logic:
change the unused variable name B to _B (e.g., "_B, L, D = sequence.size()") and
add a debug-mode check before calling torch.clamp that detects out-of-range
indices (e.g., check torch.any(indices < 0 or indices >= L) or equivalent) and
either assert or emit a warning/log via the module logger so upstream bugs
aren’t silently masked; keep the clamp as a fallback for production but ensure
the debug assertion/warning references the symbols sequence, indices, L and
torch.clamp so it’s easy to locate.
Code Review SummaryStatus: 1 Issue Found (Duplicate) | Recommendation: See existing feedback Overview
Issue Details (click to expand)WARNING
Additional Observations
Files Reviewed (1 file)
Note: The main concern about clamping silently masking bugs was already raised by sourcery-ai[bot]. Please refer to the existing inline comment for details. |
Replace Python-level bottlenecks with batched tensor operations: - decoder.py: Extract all span scores via single advanced-index call instead of N per-span .item() GPU→CPU syncs; vectorize valid-span check; cache greedy-search tuples to avoid re-creation in inner loop - modeling/utils.py: Replace O(E²) nested Python loop for entity-pair generation with torch.meshgrid + diagonal mask - data_processing/utils.py: Return LongTensor directly from prepare_span_idx using torch.arange broadcasting instead of list comprehension + later conversion - data_processing/processor.py: Replace per-element .item() dict comprehensions with single .tolist() batch conversions; update callers to consume tensor from prepare_span_idx directly All outputs verified identical across 8 equivalence tests including full end-to-end inference on CPU and GPU. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Vectorize CPU-path preprocessing and decoding hot loops
Added BibTeX entry for the Million-Label NER paper.
Replace the per-item decode loop in BaseSpanDecoder and SpanGenerativeDecoder with a single set of batch-wide tensor ops: one torch.where on the full (B,L,K,C) probability tensor, one vectorized valid-span check, one score extraction, and one batched topk for class_probs. Pure Python grouping + per-item greedy search follows. A bs=1 fast path delegates to the original per-item decoder to avoid the fixed overhead of 4D torch.where when there is nothing to amortize. GPU bs>=8: 63-95% decoder speedup (median 85%). Decoder time is nearly constant across batch sizes for short/medium inputs (~1ms at bs=8 through bs=32). CPU: improvements at very_long inputs (24-42%); regressions at short/medium where 4D torch.where has higher fixed overhead (~3-5ms absolute). Output is verified bit-identical for all 32 benchmark conditions. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…torch.where Both SpanRelexDecoder._decode_relations and TokenRelexDecoder._decode_relations used a triple-nested loop (batch × relations × classes) with per-element .item() calls. At B=32, R=500, C=10, that's 160K GPU→CPU synchronization points. Extract shared _decode_relations_batch() that: 1. Computes sigmoid and applies mask/validity in bulk tensor ops 2. Uses a single torch.where() on the full (B,R,C) probability tensor 3. Bulk-transfers results to Python via ~6 .tolist() calls 4. Groups results in pure Python with no further GPU access Total CUDA ops: ~10, regardless of B, R, or C. Bitwise identical output verified across 180K scores (100 random seeds). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…coding Vectorize relation decoding: replace B*R*C .item() calls with torch.where
Restructure the repo / update documentation
Fix normalization factor for mean loss reduction
Improve GLiNER-relex architecture
fix relex model decoding
…y_flag fix: Respect the local_files_only flag
Fix multi-task models inference
Feature/polylora
Fix docker serving related issues
Docs serving
Make the README more informative with a focus on the project itself
Update README.md
…n-lengths Fix token_lengths kwarg leaking into the labels encoder
…s-sep-tokens Preserve CLS/SEP for cls/sep-style tokenizers on transformers v5
Fix training and saving a model
Removed promotional content and updated the README structure.
There was a problem hiding this comment.
Actionable comments posted: 19
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
gliner/model.py (1)
1119-1120: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winCoerce
model_dirtoPathbefore using/.
model_diris annotated asOptional[str], but this branch treats it like aPath. A string caller will hitTypeErroratmodel_dir / "gliner_config.json"and_resolve_model_file(model_dir, ...).🛠️ Suggested fix
- if model_dir is None: + if model_dir is not None: + model_dir = Path(model_dir) + if model_dir is None: model_dir = cls._download_model(🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@gliner/model.py` around lines 1119 - 1120, Convert the optional model_dir argument to a Path before it is used in the model-loading flow, including the gliner_config.json path construction and _resolve_model_file call. Preserve the existing default-directory behavior when model_dir is None, while ensuring string callers work with Path operations.
🧹 Nitpick comments (4)
gliner/serve/Containerfile (1)
5-10: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPin dependency versions for reproducible builds.
ray[serve],transformers,huggingface_hub,safetensors, andflairare installed unpinned, so rebuilds can silently pull incompatible majors (notably Ray Serve, whose deployment API changes between minors — see the version concern ingliner/serve/server.py). Pin at leastray[serve]to a known-good version.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@gliner/serve/Containerfile` around lines 5 - 10, Pin the dependencies in the Containerfile install command to explicit, known-good versions, with particular attention to ray[serve] compatibility with the deployment API used by server.py. Apply version constraints to ray[serve], transformers, huggingface_hub, safetensors, and flair so rebuilds remain reproducible.gliner/serve/docker-entrypoint.sh (1)
4-38: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer a bash array over unquoted
$CMDword-splitting.
exec $CMDrelies on unquoted word-splitting, which breaks if any value (e.g. a model path or route prefix) contains spaces. Building an array (CMD=(python -m gliner.serve --model "${GLINER_MODEL:-...}" ...)thenexec "${CMD[@]}") is safer, and addingset -uwould surface undefined-variable typos.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@gliner/serve/docker-entrypoint.sh` around lines 4 - 38, Replace the string-based CMD construction in docker-entrypoint.sh with a bash array, preserving each argument as a separate element and quoting environment-derived values so spaces remain intact. Update every conditional option append to use array elements, invoke the command with exec "${CMD[@]}", and enable set -u to catch undefined-variable references.benchmarks/BENCHMARK_batch_level_decoding.md (1)
83-124: 🚀 Performance & Scalability | 🔵 TrivialDocumented CPU regressions look broad enough to warrant a CPU-aware fast path, not just bs=1.
The CPU results table shows large regressions for bs=8/16/32 at short and medium input lengths (e.g. -438%, -1038%, -519%), i.e. the new batched
torch.whereis markedly slower than the old per-item loop in exactly the conditions (moderate batch, short/medium sequence) that are common for CPU serving. Onlybs=1gets a fast-path fallback to the per-item decoder; consider extending that heuristic on CPU (e.g., based onB * L * Kor a CPU/GPU device check) so CPU callers don't regress at the batch sizes where the old path was already fast.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@benchmarks/BENCHMARK_batch_level_decoding.md` around lines 83 - 124, Extend the decoder’s CPU fast-path heuristic beyond batch size 1 to cover moderate batches with short or medium inputs where the batched torch.where path regresses, using a CPU device check and an appropriate B × L × K threshold. Route those cases through the existing per-item decoder while preserving the batched path for larger CPU workloads and GPU execution.benchmarks/bench_batch_decode_results.json (1)
1-3887: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider not committing raw generated benchmark output.
This file is ~3,900 lines of raw per-iteration timing samples generated by
bench_batch_decode_pr.py. Committing full reproducible benchmark output bloats the repo and will keep re-triggering PII/secret scanners (the numeric timing values are already flagged, likely falsely, as credit-card-number patterns by static analysis). Consider gitignoringbenchmarks/*_results.jsonor committing only the summarized table fromBENCHMARK_batch_level_decoding.md.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@benchmarks/bench_batch_decode_results.json` around lines 1 - 3887, Remove the generated raw benchmark artifact bench_batch_decode_results.json from version control and add benchmarks/*_results.json to the repository’s ignore rules. Preserve benchmark conclusions by retaining only the summarized results in BENCHMARK_batch_level_decoding.md, if applicable, and ensure the generated file is no longer tracked.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/release.yaml:
- Around line 12-18: Add workflow-level permissions: {} to
.github/workflows/release.yaml and .github/workflows/tests.yml; in release.yaml
grant contents: read only to checkout jobs and retain id-token: write only on
publish-to-pypi, while in tests.yml grant contents: read separately to the test
and lint jobs.
- Around line 31-32: Update the release workflow step that runs python3 -m build
to extract the built artifact’s distribution version and compare it with the
current release tag after removing its leading v. Fail the workflow when the
versions differ, while allowing the publish flow to continue only for an exact
match.
- Around line 57-63: Update the “Verify tag is on main branch” step to pass
github.ref_name through the step’s env block, then reference the environment
variable as a quoted shell argument rather than interpolating the GitHub
expression directly. Preserve the existing branch-containment check and
success/failure behavior while ensuring the tag value cannot inject shell
syntax.
In @.github/workflows/tests.yml:
- Around line 26-27: Update both checkout steps in the workflow, including the
lint checkout, to set persist-credentials to false under actions/checkout@v4.
Keep the existing repository checkout behavior unchanged.
In `@benchmarks/bench_int8.py`:
- Around line 167-171: Update the fp16 model initialization in the benchmark’s
conditions["fp16"] assignment to pass dtype="fp16" to GLiNER.from_pretrained
instead of quantize="fp16", preserving the existing model name and device
arguments.
In `@benchmarks/bench_quantize_flash.py`:
- Around line 123-142: Rename the `CONFIGS` entry currently labeled `Quantized
(fp16)` to `Quantized (int8)` so it accurately reflects the default behavior of
`model.quantize()`. Preserve its existing quantize, compile, and flash settings.
In `@compose.yml`:
- Around line 22-27: Update the runtime image built by the serve Containerfile
to install curl so the compose healthcheck command can execute successfully.
Keep the existing healthcheck URL and timing configuration unchanged.
In `@docs/usage.md`:
- Around line 1261-1265: Update the inference documentation around
model.predict_entities to clarify that precomputed prompts provide the
compressed embeddings, but callers must still pass the matching compressed label
set when invoking inference. Remove the contradictory statement that labels are
unnecessary and ensure the example and surrounding text consistently describe
this API contract.
- Around line 479-480: Update the dtype documentation near the quantization
options to describe dtype="fp16"/"bf16" as the preferred efficient loading path,
not the only way to obtain half-precision inference; preserve the existing
distinction from quantize="int8" and acknowledge the preceding
model.to(torch.float16) path.
In `@eval.py`:
- Line 10: Update the help text for the --log_dir argument in the parser
configuration to describe it as the path to the log directory, replacing the
misleading model-folder description.
In `@gliner/config.py`:
- Around line 206-209: Update the constructor augmentation defaults to match the
documented opt-in behavior: set augment_data_prob to 0.0 and align
augment_ent_drop_prob and augment_rel_drop_prob with the documented ranges,
including the entity-drop upper bound of 0.4. Keep the docstring and default
values consistent.
In `@gliner/decoding/decoder.py`:
- Around line 838-907: The new _decode_relations_batch helper is not integrated
and omits the entity_spans-to-decoded-span mapping required by
SpanRelexDecoder._decode_relations and TokenRelexDecoder._decode_relations.
Either wire both decoders to use the helper after adding the required mapping so
their existing output semantics remain unchanged, or remove the unused helper
and its misleading vectorization documentation.
In `@gliner/serve/config.py`:
- Line 33: Update the batch_wait_timeout_ms default in GLiNERServeConfig from
5.0 to 10.0 so direct configuration matches the CLI and documented behavior.
In `@gliner/serve/Containerfile`:
- Line 1: Update the Containerfile to create a dedicated non-root user, grant
that user ownership or write access to the Hugging Face cache directory, and set
the final runtime USER to that account so the serving process does not run as
root.
In `@gliner/serve/server.py`:
- Around line 596-604: Update the `@serve.deployment` configuration around the
server deployment to use Ray Serve 2.9-compatible request concurrency settings:
replace the unsupported max_ongoing_requests and max_queued_requests arguments
with max_concurrent_queries, or otherwise align the dependency version with the
newer kwargs. Preserve the existing replica and resource configuration.
In `@README.md`:
- Around line 83-86: Update the Ray Serve installation command in the “With
serving support (Ray Serve)” README section to quote the package extras in both
pip variants, preventing shell glob expansion while preserving the existing
installation options.
- Around line 109-115: Update the README output code fence to specify a language
identifier, preferably text, so the example complies with Markdownlint MD040
while preserving its contents.
In `@RELEASE.md`:
- Around line 49-52: Update both release command blocks in RELEASE.md to stage
only gliner/__init__.py instead of the entire gliner directory, while preserving
the existing commit and push commands.
In `@scripts/convert_relex_to_gliner.py`:
- Around line 29-33: Update convert_record to handle an empty
record["extraction"] list before indexing it, returning the appropriate
empty-conversion result or otherwise skipping the malformed record while
preserving the existing return contract. Ensure the conversion run does not
abort and include record-identifying context if the established flow reports
malformed records.
---
Outside diff comments:
In `@gliner/model.py`:
- Around line 1119-1120: Convert the optional model_dir argument to a Path
before it is used in the model-loading flow, including the gliner_config.json
path construction and _resolve_model_file call. Preserve the existing
default-directory behavior when model_dir is None, while ensuring string callers
work with Path operations.
---
Nitpick comments:
In `@benchmarks/bench_batch_decode_results.json`:
- Around line 1-3887: Remove the generated raw benchmark artifact
bench_batch_decode_results.json from version control and add
benchmarks/*_results.json to the repository’s ignore rules. Preserve benchmark
conclusions by retaining only the summarized results in
BENCHMARK_batch_level_decoding.md, if applicable, and ensure the generated file
is no longer tracked.
In `@benchmarks/BENCHMARK_batch_level_decoding.md`:
- Around line 83-124: Extend the decoder’s CPU fast-path heuristic beyond batch
size 1 to cover moderate batches with short or medium inputs where the batched
torch.where path regresses, using a CPU device check and an appropriate B × L ×
K threshold. Route those cases through the existing per-item decoder while
preserving the batched path for larger CPU workloads and GPU execution.
In `@gliner/serve/Containerfile`:
- Around line 5-10: Pin the dependencies in the Containerfile install command to
explicit, known-good versions, with particular attention to ray[serve]
compatibility with the deployment API used by server.py. Apply version
constraints to ray[serve], transformers, huggingface_hub, safetensors, and flair
so rebuilds remain reproducible.
In `@gliner/serve/docker-entrypoint.sh`:
- Around line 4-38: Replace the string-based CMD construction in
docker-entrypoint.sh with a bash array, preserving each argument as a separate
element and quoting environment-derived values so spaces remain intact. Update
every conditional option append to use array elements, invoke the command with
exec "${CMD[@]}", and enable set -u to catch undefined-variable references.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5975a1d0-171f-429b-bcf5-2538e16c2dc4
⛔ Files ignored due to path filters (3)
assets/banner.pngis excluded by!**/*.pngimage/GitHub.pngis excluded by!**/*.pnguv.lockis excluded by!**/*.lock
📒 Files selected for processing (69)
.github/workflows/release.yaml.github/workflows/tests.yml.gitignoreREADME.mdRELEASE.mdbenchmarks/BENCHMARK_batch_level_decoding.mdbenchmarks/bench_batch_decode_pr.pybenchmarks/bench_batch_decode_results.jsonbenchmarks/bench_gliner_e2e.pybenchmarks/bench_infer_packing.pybenchmarks/bench_int8.pybenchmarks/bench_quantize_flash.pybenchmarks/eval_compressed_biomed.pycompose.ymlconfigs/config.yamlconfigs/config_biencoder.yamlconfigs/config_decoder.yamlconfigs/config_relex.yamlconfigs/config_span.yamlconfigs/config_token.yamldocs/conf.pydocs/convert_to_onnx.mddocs/index.mddocs/instalation.mddocs/serving.mddocs/usage.mdeval.pygliner/__init__.pygliner/config.pygliner/data_processing/collator.pygliner/data_processing/processor.pygliner/data_processing/utils.pygliner/decoding/decoder.pygliner/evaluation/evaluate_ner.pygliner/evaluation/evaluator.pygliner/model.pygliner/modeling/base.pygliner/modeling/decoder.pygliner/modeling/encoder.pygliner/modeling/layers.pygliner/modeling/outputs.pygliner/modeling/span_rep.pygliner/modeling/utils.pygliner/multitask/base.pygliner/multitask/relation_extraction.pygliner/serve/Containerfilegliner/serve/__init__.pygliner/serve/__main__.pygliner/serve/client.pygliner/serve/config.pygliner/serve/docker-entrypoint.shgliner/serve/memory.pygliner/serve/server.pygliner/training/trainer.pypyproject.tomlscripts/convert_relex_to_gliner.pyscripts/jsonl_to_json.pytests/test_data_processing.pytests/test_decoder.pytests/test_features_selection.pytests/test_infer_packing.pytests/test_local_files_only.pytests/test_modeling.pytests/test_models.pytests/test_quantize_and_dtype.pytests/test_serve.pytests/test_tokenizer_stanza.pytests/utils_infer.pytrain.py
| jobs: | ||
| build: | ||
| name: Build distribution 📦 | ||
| runs-on: ubuntu-latest | ||
|
|
||
| steps: | ||
| - uses: actions/checkout@v6 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Use explicit least-privilege permissions in both workflows.
Both workflows inherit repository/org defaults, making the privileges of release and pull-request jobs configuration-dependent.
.github/workflows/release.yaml#L12-L18: add workflow-levelpermissions: {}; grantcontents: readto checkout jobs and keepid-token: writeonly onpublish-to-pypi..github/workflows/tests.yml#L16-L27: add workflow-levelpermissions: {}and grantcontents: readseparately to the test and lint jobs.
📍 Affects 2 files
.github/workflows/release.yaml#L12-L18(this comment).github/workflows/tests.yml#L16-L27
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/release.yaml around lines 12 - 18, Add workflow-level
permissions: {} to .github/workflows/release.yaml and
.github/workflows/tests.yml; in release.yaml grant contents: read only to
checkout jobs and retain id-token: write only on publish-to-pypi, while in
tests.yml grant contents: read separately to the test and lint jobs.
Source: Linters/SAST tools
| - name: Build a binary wheel and a source tarball | ||
| run: python3 -m build |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Verify that the package version matches the release tag.
pyproject.toml obtains the distribution version from gliner.__version__, but this workflow accepts any v* tag without comparing it to the built artifact. A tag such as v0.4.0 could publish a different or development version. Fail the build when the artifact version does not equal the tag version without the v prefix.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/release.yaml around lines 31 - 32, Update the release
workflow step that runs python3 -m build to extract the built artifact’s
distribution version and compare it with the current release tag after removing
its leading v. Fail the workflow when the versions differ, while allowing the
publish flow to continue only for an exact match.
| - name: Verify tag is on main branch | ||
| run: | | ||
| if ! git branch -r --contains ${{ github.ref_name }} | grep -q 'origin/main'; then | ||
| echo "Error: Tag ${{ github.ref_name }} is not on the main branch" | ||
| exit 1 | ||
| fi | ||
| echo "✓ Tag ${{ github.ref_name }} is on main branch" |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
Do not interpolate github.ref_name directly into the shell.
A user or token able to create a matching tag can inject shell syntax into this job, which has id-token: write. Pass the ref through env, quote it, and use the tag ref or commit as a quoted argument.
Proposed fix
- name: Verify tag is on main branch
+ env:
+ TAG_REF: ${{ github.ref }}
+ TAG_NAME: ${{ github.ref_name }}
run: |
- if ! git branch -r --contains ${{ github.ref_name }} | grep -q 'origin/main'; then
- echo "Error: Tag ${{ github.ref_name }} is not on the main branch"
+ if ! git branch -r --contains "$TAG_REF" | grep -q 'origin/main'; then
+ printf 'Error: Tag %s is not on the main branch\n' "$TAG_NAME"
exit 1
fi
- echo "✓ Tag ${{ github.ref_name }} is on main branch"
+ printf '✓ Tag %s is on main branch\n' "$TAG_NAME"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Verify tag is on main branch | |
| run: | | |
| if ! git branch -r --contains ${{ github.ref_name }} | grep -q 'origin/main'; then | |
| echo "Error: Tag ${{ github.ref_name }} is not on the main branch" | |
| exit 1 | |
| fi | |
| echo "✓ Tag ${{ github.ref_name }} is on main branch" | |
| - name: Verify tag is on main branch | |
| env: | |
| TAG_REF: ${{ github.ref }} | |
| TAG_NAME: ${{ github.ref_name }} | |
| run: | | |
| if ! git branch -r --contains "$TAG_REF" | grep -q 'origin/main'; then | |
| printf 'Error: Tag %s is not on the main branch\n' "$TAG_NAME" | |
| exit 1 | |
| fi | |
| printf '✓ Tag %s is on main branch\n' "$TAG_NAME" |
🧰 Tools
🪛 zizmor (1.26.1)
[error] 59-59: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[error] 60-60: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[error] 63-63: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/release.yaml around lines 57 - 63, Update the “Verify tag
is on main branch” step to pass github.ref_name through the step’s env block,
then reference the environment variable as a quoted shell argument rather than
interpolating the GitHub expression directly. Preserve the existing
branch-containment check and success/failure behavior while ensuring the tag
value cannot inject shell syntax.
Source: Linters/SAST tools
| - name: Check out repository | ||
| uses: actions/checkout@v4 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Disable persisted Git credentials in both checkouts.
These jobs execute code from pull requests, while actions/checkout stores GITHUB_TOKEN in .git/config. A malicious change can read and exfiltrate that token. Set persist-credentials: false under both checkout steps.
Proposed fix
- name: Check out repository
uses: actions/checkout@v4
+ with:
+ persist-credentials: falseApply the same change to the lint checkout.
Also applies to: 55-56
🧰 Tools
🪛 zizmor (1.26.1)
[warning] 26-27: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/tests.yml around lines 26 - 27, Update both checkout steps
in the workflow, including the lint checkout, to set persist-credentials to
false under actions/checkout@v4. Keep the existing repository checkout behavior
unchanged.
Source: Linters/SAST tools
| # fp16 | ||
| print("Loading fp16 model...") | ||
| conditions["fp16"] = GLiNER.from_pretrained( | ||
| MODEL_NAME, map_location=device, quantize="fp16" | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the relevant files first
printf '\n== benchmarks/bench_int8.py outline ==\n'
ast-grep outline benchmarks/bench_int8.py --view expanded || true
printf '\n== gliner/model.py outline ==\n'
ast-grep outline gliner/model.py --view expanded || true
# Show the exact benchmark lines around the fp16/int8 loads
printf '\n== benchmarks/bench_int8.py lines 155-185 ==\n'
sed -n '155,185p' benchmarks/bench_int8.py | cat -n
# Find from_pretrained / quantize / dtype contract in the model file
printf '\n== gliner/model.py matches ==\n'
rg -n "def from_pretrained|quantize=|dtype=|Only 'int8' is accepted|fp16|bf16|raise ValueError" gliner/model.py
# Show the relevant section around quantize() / contract docs
printf '\n== gliner/model.py lines around quantize contract ==\n'
python3 - <<'PY'
from pathlib import Path
p = Path('gliner/model.py')
lines = p.read_text().splitlines()
for i, line in enumerate(lines, 1):
if ("Only 'int8' is accepted" in line or "def quantize" in line or "fp16" in line or "bf16" in line or "from_pretrained" in line) and 500 <= i <= 620:
start = max(1, i-8)
end = min(len(lines), i+25)
for j in range(start, end + 1):
print(f"{j:4d}: {lines[j-1]}")
print("\n---\n")
PYRepository: cicero-im/GLiNER
Length of output: 38046
Use dtype="fp16" here, not quantize="fp16"
quantize() now accepts only "int8"; precision-only casts like fp16/bf16 belong on GLiNER.from_pretrained(..., dtype=...). This call will raise before the benchmark can run.
Fix
- conditions["fp16"] = GLiNER.from_pretrained(
- MODEL_NAME, map_location=device, quantize="fp16"
- )
+ conditions["fp16"] = GLiNER.from_pretrained(
+ MODEL_NAME, map_location=device, dtype="fp16"
+ )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # fp16 | |
| print("Loading fp16 model...") | |
| conditions["fp16"] = GLiNER.from_pretrained( | |
| MODEL_NAME, map_location=device, quantize="fp16" | |
| ) | |
| # fp16 | |
| print("Loading fp16 model...") | |
| conditions["fp16"] = GLiNER.from_pretrained( | |
| MODEL_NAME, map_location=device, dtype="fp16" | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@benchmarks/bench_int8.py` around lines 167 - 171, Update the fp16 model
initialization in the benchmark’s conditions["fp16"] assignment to pass
dtype="fp16" to GLiNER.from_pretrained instead of quantize="fp16", preserving
the existing model name and device arguments.
| @serve.deployment( | ||
| num_replicas=config.num_replicas, | ||
| ray_actor_options={ | ||
| "num_gpus": config.num_gpus_per_replica, | ||
| "num_cpus": config.num_cpus_per_replica, | ||
| }, | ||
| max_ongoing_requests=config.max_ongoing_requests, | ||
| max_queued_requests=config.queue_capacity, | ||
| ) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Ray Serve 2.9.0 serve.deployment max_ongoing_requests max_queued_requests parameters
💡 Result:
In Ray Serve 2.9.0, the parameter max_ongoing_requests does not exist as a deployment configuration option; instead, the functionality is controlled by the max_concurrent_queries parameter [1][2]. The max_queued_requests parameter was also not available in the 2.9.0 release [1][3]. For Ray Serve 2.9.0: - max_concurrent_queries: This parameter defines the maximum number of queries sent to a replica of a deployment without receiving a response [1][4]. It defaults to 100 in this version [1][4]. - max_queued_requests: This parameter was introduced in later versions of Ray Serve and is not a valid configuration option for the @serve.deployment decorator in version 2.9.0 [1][5][3]. In newer versions of Ray Serve (starting from version 2.32.0), the parameter max_concurrent_queries was renamed to max_ongoing_requests and its default value was changed from 100 to 5 [6][7]. Additionally, max_queued_requests was added to manage request queuing at the caller/proxy level, allowing for load shedding [6][8]. Users on version 2.9.0 should use max_concurrent_queries to manage request concurrency [1][2].
Citations:
- 1: https://docs.ray.io/en/releases-2.9.0/serve/api/doc/ray.serve.deployment_decorator.html
- 2: https://docs.ray.io/en/releases-2.9.0/serve/api/doc/ray.serve.Deployment.html
- 3: https://docs.ray.io/en/releases-2.9.2/serve/api/doc/ray.serve.schema.DeploymentSchema.html
- 4: https://docs.ray.io/en/releases-2.9.0/serve/configure-serve-deployment.html
- 5: https://docs.ray.io/en/releases-2.54.1/serve/api/doc/ray.serve.deployment_decorator.html
- 6: https://docs.ray.io/en/latest/serve/configure-serve-deployment.html
- 7: https://docs.ray.io/en/master/serve/api/doc/ray.serve.deployment_decorator.html
- 8: https://docs.ray.io/en/latest/serve/production-guide/best-practices.html
🌐 Web query:
Ray Serve 2.9.0 serve.run route_prefix parameter documentation
💡 Result:
In Ray Serve 2.9.0, the route_prefix parameter in the serve.run function determines the HTTP route prefix at which the Serve application is exposed [1]. Key details regarding the route_prefix parameter in serve.run include: - Functionality: It specifies the base HTTP path for the application. Requests sent to this prefix will be routed to the application's ingress deployment [1]. - Default Behavior: If the route_prefix is not explicitly provided, Ray Serve will attempt to use the route_prefix defined on the ingress deployment. If neither is specified, it defaults to '/' [1]. - Disabling HTTP: While not explicitly noted in the 2.9.0 API documentation as None-supportive (unlike later versions), the parameter is primarily used to control the entrypoint for HTTP traffic [1][2]. - CLI Usage: When using the serve run CLI command, the --route-prefix option can be used to set this value for applications specified via an import path; however, this option is ignored if you are deploying using a YAML configuration file [3]. - Configuration Priority: If you are using a Serve configuration (YAML) file, you should define the route_prefix within the application configuration in that file rather than relying on the serve.run parameter [4][5]. Note that in Ray Serve 2.9.0, the route_prefix should no longer be specified at the individual deployment level (e.g., via the @serve.deployment decorator), as this has been deprecated in favor of defining it at the application level via serve.run or the config file [6]. Requirements for the route_prefix include that it must start with a '/' and must not end with a '/' (unless the prefix is simply '/') [7].
Citations:
- 1: https://docs.ray.io/en/releases-2.9.0/serve/api/doc/ray.serve.run.html
- 2: https://docs.ray.io/en/latest/serve/api/doc/ray.serve.run.html
- 3: https://docs.ray.io/en/releases-2.9.0/serve/api/index.html
- 4: https://docs.ray.io/en/latest/serve/production-guide/config.html
- 5: https://docs.ray.io/en/releases-2.9.0/serve/configure-serve-deployment.html
- 6: https://docs.ray.io/en/latest/%5Fmodules/ray/serve/api.html
- 7: https://docs.ray.io/en/latest/_modules/ray/serve/schema.html
Use Ray Serve 2.9-compatible deployment kwargs
max_ongoing_requests and max_queued_requests aren’t valid on @serve.deployment in Ray Serve 2.9.0, so this will fail at deploy time. Use max_concurrent_queries instead, or upgrade Ray Serve to a version that supports the newer settings.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@gliner/serve/server.py` around lines 596 - 604, Update the `@serve.deployment`
configuration around the server deployment to use Ray Serve 2.9-compatible
request concurrency settings: replace the unsupported max_ongoing_requests and
max_queued_requests arguments with max_concurrent_queries, or otherwise align
the dependency version with the newer kwargs. Preserve the existing replica and
resource configuration.
Source: Linters/SAST tools
| **With serving support (Ray Serve):** | ||
| ```bash | ||
| !pip install gliner | ||
| uv pip install gliner[serve] # or: pip install gliner ray[serve] | ||
| ``` |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Quote package extras in shell examples.
In zsh, unquoted brackets are treated as glob patterns, so these commands can fail with no matches found. Quote both extras.
Proposed fix
-uv pip install gliner[serve] # or: pip install gliner ray[serve]
+uv pip install 'gliner[serve]' # or: pip install gliner 'ray[serve]'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| **With serving support (Ray Serve):** | |
| ```bash | |
| !pip install gliner | |
| uv pip install gliner[serve] # or: pip install gliner ray[serve] | |
| ``` | |
| **With serving support (Ray Serve):** |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@README.md` around lines 83 - 86, Update the Ray Serve installation command in
the “With serving support (Ray Serve)” README section to quote the package
extras in both pip variants, preventing shell glob expansion while preserving
the existing installation options.
| **Output:** | ||
| ``` | ||
| Cristiano Ronaldo dos Santos Aveiro => person | ||
| 5 February 1985 => date | ||
| Al Nassr => teams | ||
| Portugal national team => teams | ||
| Ballon d'Or => award | ||
| UEFA Men's Player of the Year Awards => award | ||
| European Golden Shoes => award | ||
| UEFA Champions Leagues => competitions | ||
| UEFA European Championship => competitions | ||
| UEFA Nations League => competitions | ||
| European Championship => competitions | ||
| Al Nassr => organization | ||
| Portugal => location | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add a language to the output code fence.
Use text (or another suitable language) so the README passes Markdownlint MD040.
Proposed fix
-```
+```text📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| **Output:** | |
| ``` | |
| Cristiano Ronaldo dos Santos Aveiro => person | |
| 5 February 1985 => date | |
| Al Nassr => teams | |
| Portugal national team => teams | |
| Ballon d'Or => award | |
| UEFA Men's Player of the Year Awards => award | |
| European Golden Shoes => award | |
| UEFA Champions Leagues => competitions | |
| UEFA European Championship => competitions | |
| UEFA Nations League => competitions | |
| European Championship => competitions | |
| Al Nassr => organization | |
| Portugal => location | |
| ``` | |
| **Output:** |
🧰 Tools
🪛 markdownlint-cli2 (0.23.0)
[warning] 110-110: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@README.md` around lines 109 - 115, Update the README output code fence to
specify a language identifier, preferably text, so the example complies with
Markdownlint MD040 while preserving its contents.
Source: Linters/SAST tools
| ```bash | ||
| git add gliner | ||
| git commit -m "Release: v{VERSION}" | ||
| git push origin main |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Stage only the version file.
git add gliner stages every change under the package directory, including unrelated modifications. Since the instructions only update gliner/__init__.py, stage that file explicitly in both steps.
Proposed fix
-git add gliner
+git add gliner/__init__.pyAlso applies to: 90-93
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@RELEASE.md` around lines 49 - 52, Update both release command blocks in
RELEASE.md to stage only gliner/__init__.py instead of the entire gliner
directory, while preserving the existing commit and push commands.
| def convert_record(record: dict) -> tuple[dict, int, int]: | ||
| """Convert one raw record and return it with dropped entity/relation counts.""" | ||
| text = record["text"] | ||
| extraction = record["extraction"][0] | ||
| tokens, token_spans = tokenize(text) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Unguarded index into record["extraction"] can crash on malformed records.
extraction = record["extraction"][0] will raise an unhandled IndexError (with no indication of which record or why) if extraction is an empty list, aborting the whole conversion run.
🛡️ Proposed fix
def convert_record(record: dict) -> tuple[dict, int, int]:
"""Convert one raw record and return it with dropped entity/relation counts."""
text = record["text"]
- extraction = record["extraction"][0]
+ extractions = record.get("extraction") or []
+ if not extractions:
+ raise ValueError(f"Record has no extraction entries: {record!r}")
+ extraction = extractions[0]
tokens, token_spans = tokenize(text)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def convert_record(record: dict) -> tuple[dict, int, int]: | |
| """Convert one raw record and return it with dropped entity/relation counts.""" | |
| text = record["text"] | |
| extraction = record["extraction"][0] | |
| tokens, token_spans = tokenize(text) | |
| def convert_record(record: dict) -> tuple[dict, int, int]: | |
| """Convert one raw record and return it with dropped entity/relation counts.""" | |
| text = record["text"] | |
| extractions = record.get("extraction") or [] | |
| if not extractions: | |
| raise ValueError(f"Record has no extraction entries: {record!r}") | |
| extraction = extractions[0] | |
| tokens, token_spans = tokenize(text) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/convert_relex_to_gliner.py` around lines 29 - 33, Update
convert_record to handle an empty record["extraction"] list before indexing it,
returning the appropriate empty-conversion result or otherwise skipping the
malformed record while preserving the existing return contract. Ensure the
conversion run does not abort and include record-identifying context if the
established flow reports malformed records.
8 participants
Summary by Sourcery
Bug Fixes:
Summary by CodeRabbit