fix(harness): formatValue's truncation marker could push output past VALUE_CAP - #675
Open
teyrebaz33 wants to merge 1 commit into
Open
fix(harness): formatValue's truncation marker could push output past VALUE_CAP#675teyrebaz33 wants to merge 1 commit into
teyrebaz33 wants to merge 1 commit into
Conversation
…VALUE_CAP Fourth instance of the same bug class found in the repo-wide sweep that also turned up the three fixed in sapiom#656 (normalizer.ts, record-archive.ts, resume-brief.ts) -- this one lives in the web frontend (web/src/lib/extract-step-context.ts), a different subsystem entirely (the run-inspector's Debug/Explain macro context builder), so it gets its own PR rather than joining that one. formatValue's doc comment says values are "capped at VALUE_CAP characters," but it sliced to exactly VALUE_CAP then appended a marker after the slice, so the total could exceed VALUE_CAP by the marker's length. Unlike the three fixed in sapiom#656, this marker (`\n… (truncated, N chars total)`) reports the value's original total length, which doesn't change with where the slice lands -- so unlike those three, the fix here is a single subtraction, not a converging loop: the marker's length is fixed once `text.length` is known, so there's nothing to iterate on. Updated the one existing test whose exact-match assertion had encoded the old (over-budget) output as the expected value, with the new expected value computed independently. vitest run web/src/lib/extract-step-context.test.ts: 51/51 passing. tsc --noEmit clean on both the server and web tsconfigs. `web/` is excluded from this package's eslint config (see .eslintrc.cjs's ignorePatterns) -- pre-existing, unrelated to this change. No dependency or lockfile changes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Primary change type
Problem and motivation
formatValue(packages/harness/web/src/lib/extract-step-context.ts) is documented as producing output "capped at VALUE_CAP characters," but it sliced content to exactlyVALUE_CAPand then appended a truncation marker AFTER the slice — so the total could exceedVALUE_CAPby the marker's length. This is the fourth instance of the same bug class found in a repo-wide sweep; the other three (truncateForPayload/clip/clampinpackages/harness/src/core/) are fixed in #656 — this one lives in a different subsystem (the web frontend's run-inspector Debug/Explain macro context builder), so it's a separate PR.Summary and scope
Unlike the three fixed in #656, this marker (
\n… (truncated, N chars total)) reports the value's original total length, which doesn't change with where the slice lands — so the fix here is a single subtraction, not a converging loop: the marker's length is fixed oncetext.lengthis known, so there's nothing to iterate on. Reserve the marker's length withinVALUE_CAPbefore slicing.Out of scope: the three
packages/harness/src/core/instances (separate PR, #656); no dependency changes.Related work
Related issue or discussion: #674
Validation
Tests and documentation
Updated the one existing test whose exact-match assertion had encoded the old (over-budget) output as the expected value, with the new expected value computed independently. No user-facing documentation changes needed.
Compatibility and release impact
formatValueis not exported;extractStepContext's signature and return type are unchanged. Truncated output near the cap gets slightly shorter (by the marker's length) to actually respect the documented bound..changeset/fix-formatvalue-budget-overrun.md), patch bump for@sapiom/harness.Security
will follow the
Security Policy for
private reporting.
AI assistance
I used Claude (Anthropic) as a coding assistant throughout: it helped find the bug (via a repo-wide grep sweep for the same marker pattern), draft the fix and the updated test, and run the verification commands quoted above. I reviewed and ran every command myself, read and understood the resulting diff line by line, and can explain and maintain every change in this PR.
Checklist
CONTRIBUTING.md, and this contribution follows the direct-PR or issue-first policy.any N/A checks above.