fix(collector): treat a blank published value as absent, not as the epoch - #6275
Open
marmar9615-cloud wants to merge 1 commit into
Open
fix(collector): treat a blank published value as absent, not as the epoch#6275marmar9615-cloud wants to merge 1 commit into
marmar9615-cloud wants to merge 1 commit into
Conversation
…poch
GET /v1/document/metadata-schema documents this key as
"epoch timestamp in ms | nullable", but Number(null), Number("") and
Number(" ") are all 0, so an explicitly null published stamps the document
1/1/1970 (12/31/1969 west of UTC).
That date is not only cosmetic. The value is plucked into chunkHeaderMeta and
stringifyHeader prepends it as literal <document_metadata> text on every chunk,
which all eight vector providers write, so the model reads the wrong date as
part of the document body.
Guard on a trimmed string so null, "", whitespace and [] are all treated as
absent. published: 0 is a real epoch value and still resolves to 1970.
undefined needs no clause: Number(undefined) is NaN, so it already fell to the
isNaN branch.
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.
Summary
Resolves #6274.
GET /v1/document/metadata-schemadocumentspublishedas"epoch timestamp in ms | nullable", but sending it asnullstamps the document with the Unix epoch.collector/processRawText/index.js:42Number(null),Number("")andNumber(" ")are all0, so theisNaNguard never fires and the value becomes1/1/1970(12/31/1969west of UTC). Omitting the key works, becauseNumber(undefined)isNaN.Why this is worth more than a wrong date column
publishedis plucked intochunkHeaderMeta(server/utils/TextSplitter/index.js:73-76) andstringifyHeader()prepends it as literal<document_metadata>text on every chunk (:135-147), called unconditionally by all eight vector providers. So the epoch date is embedded into the text the model reads. That path is traced through the source rather than exercised here: the tests in this PR assert the collector'spublishedvalue, not the rendered chunk header. In the UI it only surfaces in the file row's hover tooltip (Directory/FileRow/index.jsx:12-17), which is the smaller half of the problem.Change
Five inserted lines in one file, three of them comment. Guard on a trimmed string so
null,"", whitespace and[]are all absent.published: 0is a genuine epoch timestamp and still resolves to 1970. No clause is needed forundefined:Number(undefined)is alreadyNaN, so it falls to the existingisNaNbranch. I checked that rather than adding a clause that does nothing.Tests
Nine cases in
collector/__tests__/processRawText/index.test.js, run against three variants ingit archiveexports so the working tree was never mutated:masternull,""," ","\t",[]if (!published)published: 0, and still misses" ","\t",[]That middle row is why the guard trims rather than testing truthiness.
!publishedlooks equivalent and is not: it treats a legitimate0epoch as absent while letting whitespace through.The four cases that pass on
masterare controls that must keep passing: an omitted key, a numeric epoch, a string epoch, and0.npx jest collectoron this branch: 157 passed, 10 suites, against a 148 passed, 9 suites baseline onmaster. Prettier clean.The nine new tests discriminate rather than merely pass:
master(unfixed)if (!published)instead of the trimmed-string guard" ","\t"and[]are all truthy and fall through toNumber()Repository-wide,
yarn testis red onmasterbefore this change: 21 suites fail on Node 26 wherejsonwebtokenreaches for the removedSlowBuffer. That is unrelated to this PR and this PR does not touch it, which is why the numbers above are scoped tonpx jest collector.One case deliberately left
published: falsestill resolves to 1970, becauseNumber(false)is0and`${false}`.trim()is"false". A boolean is outside the documented type for this field, and separating it from0needs atypeofcheck for an input nothing sends. Happy to add it if you would rather the guard be total.