Skip to content

fix(openai): guard truncation loop from emptying the user slot on token-dense standups - #72

Open
detail-app[bot] wants to merge 1 commit into
mainfrom
detail/bug-fix/fix-openai-guard-truncation-loop-from-emptying-the-927afb
Open

detail-app[bot] wants to merge 1 commit into
mainfrom
detail/bug-fix/fix-openai-guard-truncation-loop-from-emptying-the-927afb

Conversation

@detail-app

@detail-app detail-app Bot commented Sep 25, 2026

Copy link
Copy Markdown
Contributor

Detail bug report: View on Detail

Bug

  • textToCompletionPrompt (src/openai/format.ts) truncates standup text so the user slot fits MODEL_MAX_TOKENS. Its loop computes the keep-count as words.length - Math.ceil((tokens - MODEL_MAX_TOKENS) / 2).
  • For token-dense, space-sparse input (e.g. compact JSON.stringify output or a base64 blob — thousands of tokens but only a handful of space-delimited words), this keep-count drops to 0 or below -words.length, so slice(0, end) returns [] and the user message collapses to "".
  • Introduced by 7f5e346 (which fixed a prior infinite loop but overshot). Only the fine-tuning training path is affected — it has no length guard before truncation. The live-scoring path short-circuits text.length > 1000 before this code runs, so it's unaffected.
  • Effect: a human-rated standup over budget at high token density yields a training example pairing an empty standup with a yes/no label, which trainModel.ts writes to the JSONL with no local validation.

Fix

  • Clamp the keep-count so the user slot is never emptied: floor at 1, cap at words.length.
  • Break the loop when an iteration makes no progress (next >= truncatedTextTokenLength), so a single over-budget word that can't be reduced further terminates instead of infinite-looping — preserving the termination property 7f5e346 introduced.

Testing

  • Unit tests (src/openai/format.test.ts, offline custom harness) — all pass:
    • Added a compact-JSON regression test asserting the input is in the collapse region, the user slot stays non-empty, and the result fits the budget.
    • Added a single-over-budget-word test asserting the loop terminates within a time budget and the user slot stays non-empty.
    • Pre-existing tests (oversized prose, under-limit passthrough, prompt structure, classification prompts, training-example labels) still pass.
  • Typecheck (tsc --skipLibCheck --noEmit) passes.
  • Lint: only a pre-existing @typescript-eslint/parser vs TypeScript 5.2 incompatibility surfaces (a 0:0 parser error affecting every .ts file, including unchanged ones); no new violations from this change.
  • Reproduction check (unversioned, run locally): the bug report's compact-JSON standup previously produced an empty user message; after the fix it yields non-empty content within the token budget.
  • Near-end-to-end training check (unversioned, run locally): exercised trainModel's full JSONL-writing path via the same typedi injection seam the existing aiScoreStandups test uses; the generated .jsonl has no line with an empty user content when the corpus includes a collapse-triggering standup.
  • Scoring regression: the existing offline aiScoreStandups test still passes — no regression in the path that calls getProjectStandupScore.
  • Live OpenAI upload: attempted against api.openai.com but it returned 401 invalid_api_key — no real API key is available in this environment, so the accept/reject behavior of a real fine-tune upload could not be verified. The local defect (empty user content reaching the upload) is eliminated by the fix and confirmed by the near-end-to-end JSONL check above.

Automatic Fixes PRs can be configured here.

@augmentcode

augmentcode Bot commented Sep 25, 2026

Copy link
Copy Markdown
Contributor
🤖 Augment PR Summary

Summary: This PR hardens OpenAI standup prompt truncation for token-dense, space-sparse input.

Changes:

  • Clamps the word keep-count to retain at least one word and no more than the input contains.
  • Stops the truncation loop when an iteration does not reduce the token count.
  • Adds offline regression coverage for compact JSON and a single oversized whitespace-free word.
  • Retains existing tests for normal truncation, prompt structure, and training labels.
Technical note: The affected formatting is used when serializing fine-tuning examples and when creating classification prompts.

🤖 Was this summary useful? React with 👍 or 👎

@augmentcode augmentcode Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review completed. 1 suggestion posted.

Fix All in Augment

Comment augment review to trigger a new review at any time.

Comment thread src/openai/format.ts
if (keep > words.length) keep = words.length;
truncatedText = words.slice(0, keep).join(' ');
const next = encode(truncatedText).length;
if (next >= truncatedTextTokenLength) break;

@augmentcode augmentcode Bot Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

For a single whitespace-free value whose token count exceeds MODEL_MAX_TOKENS, next equals the original count, so this break returns that still-oversized value. getTrainingExample serializes it directly into the fine-tuning JSONL, so this reintroduces an invalid over-budget training example (and similarly lets scoring requests exceed the function's documented limit).

Severity: medium

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Checked src/openai/format.ts:82-91 and the PR's own testSingleOverBudgetWordTerminatesAndStaysNonEmpty (src/openai/format.test.ts:154-167), which deliberately asserts an unsplittable over-budget single word is preserved verbatim and stays over MODEL_MAX_TOKENS (the comment states "the whole word is preserved since it cannot be split, even though it remains over the model's token budget").

I attempted to fix this by hard-truncating to the token budget once word-splitting can no longer reduce the count (sub-word cut via decode(encode(text).slice(0, MODEL_MAX_TOKENS))). That brings the over-budget single word under MODEL_MAX_TOKENS, but it directly fails the PR's assertEqual(userContent, text, '... preserved verbatim ...') contract, so landing it would rewrite that test and change the PR's stated purpose. For a single whitespace-free over-budget token, "non-empty" and "under budget" are mutually exclusive under word-based truncation; getting under budget requires sub-word splitting, which the PR deliberately rejects.

Leaving the code unchanged pending a human decision: (a) keep the current behavior — preserve non-empty verbatim and accept that an unsplittable over-budget input exceeds MODEL_MAX_TOKENS in getTrainingExample/scoring, or (b) change the PR's purpose to hard-truncate unsplittable over-budget input to the token budget (which rewrites testSingleOverBudgetWordTerminatesAndStaysNonEmpty).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants