Repository navigation
fix(vector): VECTOR(N) columns round-trip, and the case-exact doc-link guard that greens the ubuntu CI leg - #448
Merged
Merged
Conversation
…Single[]" The vector case was missing from every per-type row codec, from five value tokenizers and from both parameter binders, so a float[] written to a VECTOR(N) column fell through to `default` and was persisted as the literal text "System.Single[]"; a bracketed literal failed earlier still, in the VALUES tokenizer, as "Invalid value '[0.5' for data type Vector". Fixed by giving the format and the coercion one definition, SharpCoreDB.DataStructures.VectorBinaryCodec (the VEC\0 blob per docs/Vectors/TECHNICAL_SPEC.md section 7, the [int32 length][blob] framing the row codecs store, and the accepted input forms), and routing every copy through it: Table.Serialization, FixedWidthCodec/FixedWidthRecordLayout, StructRow (write plus both readers), Table.ParallelScan/StructScanning, InsertBatchOptimized, StreamingRowEncoder, BinaryRowDecoder, BulkInsertValuePipeline, PageBasedDataWriter, ColumnarDataReader, SingleFileTable's JSON row reader, both binders, and VectorSearch.VectorSerializer, which now delegates to it. Also fixed, found by reading the same paths: the fixed-width codec's string fast path wrote raw UTF-8 into a VECTOR slot the reader decodes as a blob, and the client reader advertised double[] where every other surface produces float[]. Red before: 8 of 9 VectorColumnStorageTests cases (artifacts/vector-red3.txt). Green after: 28 vector tests, the 248-test VectorSearch suite, and the wider core suite 2032 / 0 failed. Evidence: docs/performance/WORKLOG.md, session 84.
…suites, 3533/0), the corrected landing path (PR #447 already merged; the fix lands as its own PR) and the owner hand-off helper
…nch pushes allowed, landing on master stays an owner action
…ication (2420/0, gates and SIMD green) and the two BLOCKED dependency findings (15 npm advisories; SixLabors.ImageSharp 3.1.11 outside the CI slnf)
…-- the first attempt landed mid-file inside the session-84 addendum, because Measure-Object -Line counts non-empty lines only (3762 reported vs 5142 physical)
…nch deletions with their SHAs and the ancestry proof that each was redundant
…rs/README.md Two links in the Support & Resources section of docs/Vectors/VECTOR_MIGRATION_GUIDE.md read ../vectors/README.md while the directory is docs/Vectors. Windows and macOS resolve that difference, a case-sensitive host does not -- which is why the ubuntu leg of CI/CD Pipeline failed on master at 204d9f8 and the coverage job withheld its verdict with it: Collection: ["Vectors/VECTOR_MIGRATION_GUIDE.md:793 -> ../vector...", "Vectors/VECTOR_MIGRATION_GUIDE.md:794 -> ../vector..."] Every other vector link in docs/ already uses ../Vectors/README.md.
…runner does File.Exists/Directory.Exists are case-insensitive on Windows and macOS, so the link guard added by bdb4e09 passed on those legs and failed only on ubuntu-latest -- which is how master went red unnoticed, and why the audit that added the guard could not see it. Both copies of that resolution (the link sweep and DocumentationIndex_ShouldOnlyReferenceExistingDocumentationFiles) now walk the path from the repository root and match every segment ordinally, so a wrong-case link fails on any host. Red first: with only the helper in place the test failed locally with exactly the two entries the ubuntu log shows (VECTOR_MIGRATION_GUIDE.md:793 and :794); it passes after the link fix in the previous commit. Verification, CI's own filter (Category!=Debug&Category!=Manual&Category!=Performance): core 2032/0, VectorSearch 248/0, EF Core 116/0, linq2DB 24/0 -- 2420 tests, 0 failed.
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
|
MPCoreDeveloper
added a commit
that referenced
this pull request
Oct 12, 2026
…ed, the patch-status item is refuted as already green, and codecov/project is BLOCKED The session's finding is that the work the item was opened for was not needed: PR #448 was already merged (`b6bcf34a`) and `codecov/patch` on it had already passed (51,01 % against the effective 45 % floor -- `target: 50 %` + `threshold: 5 %`), so no test was written for the 121 patch misses. What was unfinished was the config half, and that is what `4eafadd8` lands; this entry records the live-API evidence (including the `coverage: 0.0` correction to the draft comment), the runner-verified green of PR #460, and the BLOCKED `codecov/project` gap that is carried forward in NEXT rather than dropped.
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
1 participant
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.
What this lands
1.
VECTOR(N)columns round-trip (session 85). TheDataType.Vectorcase was missing fromeight copies of the per-type row codec, five value tokenizers, the single-file JSON reader and both
parameter binders, so a
float[]was persisted as the literal text"System.Single[]". Fullbreakdown in
docs/CHANGELOG.md→[Unreleased] → Fixed.2. The red ubuntu leg of
CI/CD Pipelineonmasteris fixed here (session 85, addendum 2).masterat204d9f8a— this branch's own merge — failedBuild and Test (ubuntu-latest):Two layers, both fixed:
docs/Vectors/VECTOR_MIGRATION_GUIDE.mdpointed its resource links at../vectors/README.mdwhile the directory isdocs/Vectors— fine on Windows and macOS, 404 on acase-sensitive host. They now read
../Vectors/README.md, the form every other vector link indocs/already used.File.Exists/Directory.Existsarecase-insensitive on Windows and macOS, so the link test added by this branch's own documentation
audit (
bdb4e094; absent at92794084, the last green master) could not fail on the windows ormacos leg — it failed only on ubuntu, after the merge. Both copies of that resolution (the link
sweep and
DocumentationIndex_ShouldOnlyReferenceExistingDocumentationFiles) now go through onePathExistsWithExactCasehelper that matches every path segment ordinally, so a wrong-case linkfails locally too.
Coverage (net11.0)was collateral, not a second defect:tools/measure-coverage.ps1exited 2 onthat same single failure (
FAILED SUITES: SharpCoreDB.Tests - coverage verdict withheld), so the18 % floor was never evaluated. No other job failed — windows and macos were green, and
Compatibility Smoke Testspassed.Verification
Red → green on Windows, which is the point of the guard change:
:793,:794)DocumentationConsistencyTests9 / 0Build: 0 errors. CI's own filter everywhere:
Category!=Debug&Category!=Manual&Category!=Performance.And on the runners, which is the verdict that counts here —
CI/CD Pipelinerun38163780903(head62908c7a) completed with every job success:Build and Teston windows-latest, ubuntu-latest andmacos-latest, plus
Coverage (net11.0).Compatibility Smoke Testssucceeded as well. The coverage jobno longer exits 2 with a withheld verdict; it evaluates the floor again:
This PR is left open and green for the owner to merge.
Merge notes
masterrunsPack NuGet PackagesandPublish to NuGet.org(verified on theprevious green master push, run
38130514269, where both succeeded). This PR does not merge itselffor exactly that reason — merging is the owner's action.
BLOCKEDdependency items (Dependabot's 15 npm alerts in the server-script client'spackage-lock.json, and the transitiveSixLabors.ImageSharp 3.1.11advisory in the Orchard Coreexample) remain outside this PR: they need a package-version decision, not a code change.
docs/performance/WORKLOG.md, session 85 and its two addenda.