Skip to content

fix: enforce tiny skills token budgets - #4415

Merged
IsmaelMartinez merged 2 commits into
The-PR-Agent:mainfrom
utsab345:hunt/fresh-defect
Oct 27, 2026
Merged

IsmaelMartinez merged 2 commits into
The-PR-Agent:mainfrom
utsab345:hunt/fresh-defect

Conversation

@utsab345

@utsab345 utsab345 commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

User description

A skills token budget smaller than the truncation marker currently produces output above the configured limit, which can overflow prompts even when the setting is valid. This change returns a safely clipped marker for tiny budgets and adds regression coverage to keep the formatted context within the requested token limit.


PR Type

Bug fix, Tests


Description

  • Clip truncation markers to tiny token budgets.

  • Add regression coverage for one-token skill contexts.


File Walkthrough

Relevant files
Bug fix
skills_loader.py
Keep truncated skills context within tiny budgets               

pr_agent/algo/skills_loader.py

  • Handle positive budgets smaller than the truncation marker.
  • Clip the marker without adding an ellipsis.
  • Remove trailing characters until the token count fits, then return
    early.
+5/-0     
Tests
test_skills_context_budget.py
Test skills context against one-token budgets                       

tests/unittest/test_skills_context_budget.py

  • Add a regression test with a one-token budget.
  • Assert formatted skills context stays within that limit.
+6/-0     


⚙️ Agent run details
  • Model: openai/gpt-6.1-sol
  • Tokens: 4,730 in / 261 out / 4,991 total
  • Time cost: 5.1s
  • AI calls: 1

@the-pr-agent-bot

Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ No major issues detected

⚙️ Agent run details
  • Model: openai/gpt-6.1-sol
  • Tokens: 5,442 in / 94 out / 5,536 total
  • Time cost: 4.9s
  • AI calls: 1

@IsmaelMartinez IsmaelMartinez left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @utsab345 for spotting that the truncation marker can overshoot a 1 to 3 token budget. Rather than a clipped marker like "\n\n[t", which is no use in a prompt, the docstring already promises an empty string when nothing fits. One ask: replace the new block with the guard below, and update the test to expect "".

Comment thread pr_agent/algo/skills_loader.py Outdated
Comment on lines +279 to +283
if max_tokens < marker_tokens:
marker = clip_tokens(truncate_marker, max_tokens, add_three_dots=False)
while marker and _count_tokens(marker) > max_tokens:
marker = marker[:-1]
return marker

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if max_tokens < marker_tokens:
marker = clip_tokens(truncate_marker, max_tokens, add_three_dots=False)
while marker and _count_tokens(marker) > max_tokens:
marker = marker[:-1]
return marker
if max_tokens < marker_tokens:
return ""

@IsmaelMartinez IsmaelMartinez left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @utsab345 for switching to the empty-string guard so a tiny budget no longer emits a clipped marker. Output now stays within budget for every value from 0 to 20, and the new test fails without the fix. Approving and merging.

@IsmaelMartinez
IsmaelMartinez merged commit dff634a into The-PR-Agent:main Oct 27, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants