Repository navigation
Pin what the review's Bot line keeps, not only its length - #1980
Merged
Merged
Conversation
`REVIEW_OTHER_LINE_MAX` was guarded from above alone: the ThinkFlowLab#1918 case asserted the line ends with `…(cut)` and stays within 800 + the suffix, so lowering the limit to 700 cut a hundred units off what a Bot's line shows the memory reviewer and every test stayed green. The case now pins the kept head as well — `Writer: ` plus the 791 x's, and a kept width within ten units of the limit — in the shape the neighbouring user-line case uses, so 700 goes red while 800 stays green and 900 still fails on the missing suffix. Test-only: `memory.js` keeps 800.
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.
Fixes #1978
Owner: Dev-4
Written-by: Dev-4
Reviewer: Birfy
Claim-Id: dcaa85cc5e5f4953ab2cf28d0b61735e
Branch: dev/4-p284-memory-bot-line-bound
Base: 90b880e
Tip SHA: 41cc198
Verified commit: 41cc198 (same tree, verified locally at that commit)
Tree: 730ec9f2858bb77d2b62cfb7f41be968a73d4252
Diff: 2 files, +11/−1
What changed
core/test/memory.test.js:541asserted the review's Bot line only from above —endsWith('…(cut)')at:561and<= 800 + suffixat:562— soREVIEW_OTHER_LINE_MAX(core/src/host/memory.js:64) could drop from 800 to 700 and take a hundred units of a Bot's line out of what the memory reviewer reads, silently. The case now pins what survives, in the shape of the neighbouring user-line case (:600):Writer:followed by the 791 x's — at 800 the cut steps back off the emoji's surrogate pair, so the whole kept head is those 791 units; at 700 only 692 survive;…(cut)suffix) stays within ten units of the limit, so a shrunken limit fails instead of passing on the upper bound alone.#1918's properties stay asserted and unchanged:prompt.isWellFormed(), no\uFFFD,endsWith('…(cut)'), and the<= 800 + suffixupper bound. Test-only:memory.jskeepsREVIEW_OTHER_LINE_MAX = 800. The generated mirror ispatches/ds-bot/0001-core-fixes.patch(there is nopatches/ds-bot/files/test/memory.test.jsat this base), rewritten bynode scripts/ds-bot-patches.mjs generate, never hand-edited, andcanonicalreportsgenerate is a no-op.Red first
The hole, measured now on
90b880e7with the base's own case andmemory.js:64mutated to 700:With the strengthened case, the same 700 mutation:
memory.jswas restored withgit checkout --after every mutation; its sha256 before and after is the same,689756999f0e8136e067048d0b67c44fe0a43052c552df5972c7045906bb673a, andgit status --porcelainthen names only the two files of this change.The other direction, 900, is red as before — the suffix is not there, so the original assertion at
:561fires withexpected: true / actual: falseand# tests 65 / # pass 64 / # fail 1. At 800 the file is green:# tests 65 / # pass 65 / # fail 0.Verification at the tip
Ran:at41cc198fa1faa3a78249b89fac73ccab16395aa9(tree730ec9f2858bb77d2b62cfb7f41be968a73d4252), tree clean before and after:memory.test.js65/65).github/scripts563/563, interop 37/37, live-e2e 24/24arch-docs: OK (2 changed file(s))withARCH_DOCS_BASE=90b880e7check:ok ds-bot/ = Fei2411/DeepSeek-Bot@2b10c70 (tree 5834c00, v0.1.5)/ok 10 patches apply cleanly in order/ok core/ equals ds-bot/ + patches/ds-bot/canonical:ok patches/ds-bot/ is what generate writes (generate is a no-op)Not run:the smoke bench (noindex.jsorsrc/host/change), the CIpackagejob, and the merge-queue rebuild — those run in CI and on the queue entry.{ "pr": 1980, "claim": "Dev-4", "issue": 1978, "summary": "The review-context case for a Bot's line asserted only that the line was cut and no longer than 800 plus its suffix, so REVIEW_OTHER_LINE_MAX could fall from 800 to 700 and silently drop a hundred units of context. It now also pins what survives: the kept head is `Writer: ` plus the 791 x's, and the kept width stays within ten units of the limit, in the shape of the neighbouring user-line case. Test-only; memory.js keeps 800.", "files": ["core/test/memory.test.js", "patches/ds-bot/0001-core-fixes.patch"], "checks": { "ran": "TZ=UTC ARCH_DOCS_BASE=90b880e7 npm run verify; node scripts/ds-bot-patches.mjs check; node scripts/ds-bot-patches.mjs canonical", "result": "green", "at": "41cc198fa1faa3a78249b89fac73ccab16395aa9" }, "next": "none — ready for review" }