Add support for splits on categorical features#120
Conversation
📝 WalkthroughWalkthroughVersion constant is bumped from Changesv1.1.0 Release: categorical split support docs, test, and version bump
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![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 introduces support for categorical splits in XGBoost models, updating the parser, templates, documentation, and tests accordingly. The review feedback suggests preventing potential integer overflow in slice bounds checks within gen/parse.go by using subtraction, and removing a redundant random_state parameter in the Python test model generator since a global random seed is already configured.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@README.md`:
- Around line 20-23: The README.md documentation is making a promise about nil
handling for categorical features that is not yet implemented in the code. The
generator in gen/templates.go:57-73 directly dereferences *data[i] for
categorical predicates without nil checks, which would cause a panic if a nil
entry is passed. Either remove the claim about nil entry handling from the
README and scope it to numeric features only, or first add nil-safe handling to
the categorical predicate code in gen/templates.go before documenting that
feature.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f7e3096b-aba3-4748-a379-5bc33820cad2
⛔ Files ignored due to path filters (10)
gen/parse.gois excluded by!**/gen/**gen/parse_test.gois excluded by!**/gen/**gen/templates.gois excluded by!**/gen/**gen/templates/decision_node.tmplis excluded by!**/gen/**gen/templates_test.gois excluded by!**/gen/**gen/testdata/categorical/generate-model.pyis excluded by!**/gen/**gen/testdata/categorical/model.jsonis excluded by!**/gen/**gen/testdata/categorical/preds.csvis excluded by!**/*.csv,!**/gen/**gen/testdata/categorical/xtest.csvis excluded by!**/*.csv,!**/gen/**mise.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
CHANGELOG.mdREADME.mdmain_test.goversion.go
ndawe
force-pushed
the
ndawe/categorical-features
branch
from
8dd46e9 to
fb92b28
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
README.md (1)
20-23:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't promise
nilhandling for categorical splits yet.This still conflicts with
gen/templates.go:57-73, which dereferences*data[i]for categorical predicates. Anilentry will panic, so either remove thenilclaim or add nil-safe categorical handling first.🤖 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 20 - 23, The README.md documentation claims that missing categorical features can be represented by nil entries in the data slice, but the template code in gen/templates.go at lines 57-73 dereferences pointers without nil checks for categorical predicates, which would cause a panic if nil is passed. Either remove the statement about nil handling for categorical splits from the README documentation (lines 20-23), or implement nil-safe categorical predicate handling in the template code before documenting the feature.
🤖 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.
Duplicate comments:
In `@README.md`:
- Around line 20-23: The README.md documentation claims that missing categorical
features can be represented by nil entries in the data slice, but the template
code in gen/templates.go at lines 57-73 dereferences pointers without nil checks
for categorical predicates, which would cause a panic if nil is passed. Either
remove the statement about nil handling for categorical splits from the README
documentation (lines 20-23), or implement nil-safe categorical predicate
handling in the template code before documenting the feature.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 8dab7760-6753-492c-be1a-3d04dc2d0a8b
⛔ Files ignored due to path filters (10)
gen/parse.gois excluded by!**/gen/**gen/parse_test.gois excluded by!**/gen/**gen/templates.gois excluded by!**/gen/**gen/templates/decision_node.tmplis excluded by!**/gen/**gen/templates_test.gois excluded by!**/gen/**gen/testdata/categorical/generate-model.pyis excluded by!**/gen/**gen/testdata/categorical/model.jsonis excluded by!**/gen/**gen/testdata/categorical/preds.csvis excluded by!**/*.csv,!**/gen/**gen/testdata/categorical/xtest.csvis excluded by!**/*.csv,!**/gen/**mise.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
CHANGELOG.mdREADME.mdmain_test.goversion.go
1 participant
Summary by CodeRabbit
New Features
Documentation
Tests