Skip to content

fixes stray invalid variable access bug#668

Merged
RobotSail merged 1 commit intomainfrom
os/fix-var-access-bug
Nov 18, 2025
Merged

fixes stray invalid variable access bug#668
RobotSail merged 1 commit intomainfrom
os/fix-var-access-bug

Conversation

@RobotSail
Copy link
Copy Markdown
Member

@RobotSail RobotSail commented Nov 13, 2025 •
edited by coderabbitai Bot
Loading

The analyze_dataset_statistics function is using args.max_seq_len when reporting an error instead of the max_seq_len arg that it was given. This causes API-based data processing to break since the global args is only defined when we parse out the CLI arguments.

Summary by CodeRabbit

  • Refactor
    • Clarified an internal error message relating to dataset token-length validation so it references the correct local parameter, improving clarity when encountering dataset size/sequence issues.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Nov 13, 2025 •
edited
Loading

Caution

Review failed

The pull request is closed.

Walkthrough

Error message in analyze_dataset_statistics updated to reference the local max_seq_len parameter instead of args.max_seq_len, removing an external object reference; no control-flow or behavior changes.

Changes

Cohort / File(s) Summary
Error message reference fix
src/instructlab/training/data_process.py		 Updated RuntimeError message in analyze_dataset_statistics to use local parameter max_seq_len instead of args.max_seq_len

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

  • Single-line string change only; no logic, control-flow, or API surface changes.
  • No additional files require attention.

Poem

I’m a rabbit with a keen-eyed knack,
I hopped in code and tightened a tack.
A message now points where it ought,
No outer fetch, no tangled thought.
Tiny tidy fix — a quiet hack 🐇✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fixes stray invalid variable access bug' accurately describes the main change: correcting an incorrect variable reference (args.max_seq_len to max_seq_len) in the analyze_dataset_statistics function.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 21efaee and 1e112cc.

📒 Files selected for processing (1)
  • src/instructlab/training/data_process.py (1 hunks)

Comment @coderabbitai help to get the list of available commands and usage tips.

@mergify mergify Bot added the ci-failure label Nov 13, 2025
@RobotSail
Copy link
Copy Markdown
Member Author

RobotSail commented Nov 18, 2025

@mergify rebase

@mergify
Copy link
Copy Markdown
Contributor

mergify Bot commented Nov 18, 2025

rebase

✅ Branch has been successfully rebased

@RobotSail RobotSail force-pushed the os/fix-var-access-bug branch from 21efaee to 1e112cc Compare November 18, 2025 20:06
@RobotSail RobotSail merged commit 781c36f into main Nov 18, 2025
11 of 17 checks passed
@RobotSail RobotSail deleted the os/fix-var-access-bug branch November 18, 2025 20:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

ci-failure

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@RobotSail