review/content-lane: duplicates.ts's block-scalar detection lags source-evidence.ts's hardened version (comment-tolerance, digit order) - #9366
Conversation
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9366 +/- ##
==========================================
+ Coverage 75.38% 75.46% +0.08%
==========================================
Files 275 276 +1
Lines 58023 58227 +204
Branches 6181 6263 +82
==========================================
+ Hits 43739 43943 +204
Misses 14014 14014
Partials 270 270
Flags with carried forward coverage won't be shown. Click here to find out more.
|
5eb3bfb to
40b0340
Compare
…source-evidence.ts Harden parseSimpleFrontmatter and findDuplicateFrontmatterKeys to match source-evidence.ts (|2-, comment-trailing headers). Add unit tests. Fixes JSONbored#9290 Co-authored-by: Cursor <cursoragent@cursor.com>
|
Tip ✅ LoopOver review result - approve/merge recommendedReview updated: 2026-07-27 15:23:24 UTC
Review summary Nits — 4 non-blocking
Decision drivers
Context & advisory signals — never blocks the verdict
Linked issue satisfactionAddressed Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://loopover.ai/docs/loopover-commands 🧪 Experimental — new and may change. 🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.
|
Summary
src/review/content-lane/duplicates.tshas two call sites (parseSimpleFrontmatterat line ~77and
findDuplicateFrontmatterKeysat line ~132) that detect a YAML block-scalar header (|,>,|-,|2-, etc.) using:The sibling parser in
src/review/content-lane/source-evidence.tswas previously hardened (in#8016) into a shared, more correct check:
This hardened version (a) accepts the indentation digit before or after the chomping indicator
(
|2-as well as|-2), and (b) strips a trailing inline comment first, so| # sources belowis still recognized as a block-scalar header.
duplicates.ts's regex has neither fix.Concretely,
duplicates.tsfails to recognize:|2-(digit-then-chomp order), and anyblock-scalar header with a trailing comment (
| # note,|- # note,|2- # trailing). When thathappens, the header line collapses to the literal string
"|"and the real indented block contentthat follows is silently skipped by the parser loop (indented lines don't match the
key:regex).This directly undermines
duplicates.ts's own stated purpose (per its file-level doc comment: "aregex parser that silently drops block/folded/list values would let a contributor hide a
protected-field edit and bypass the protected-edit + duplicate gates"). A protected field written
as
description: | # notefollowed by indented content would parse to"|"both before and afteran edit, so
protectedFrontmatterChangeswould never detect the real content change, and twosemantically-different entries using this style would collide as "same normalized description" in
the duplicate detector.
This is the same cross-file divergence class already fixed twice between these exact two files:
#7250 and #8016 (both closed) — this issue is the same class of gap, this time in the
opposite direction (
source-evidence.tsis ahead,duplicates.tslags).Deliverables
duplicates.ts'sparseSimpleFrontmatterrecognizes|2--style and comment-trailingblock-scalar headers identically to
source-evidence.ts.duplicates.ts'sfindDuplicateFrontmatterKeysrecognizes the same headers identically.duplicates.tstest file undertest/unit/) covering: a|2-header, a|- # commentheader, and a> # commentheader, each asserting theindented block content that follows is correctly captured (for
parseSimpleFrontmatter) orcorrectly skipped without producing a spurious duplicate/parse error (for
findDuplicateFrontmatterKeys).All three deliverables are required in this single PR.
Test plan
This repo enforces 99%+ Codecov patch coverage, branch-counted, on every changed line/branch in
src/**. The new test cases above must exercise both fixed call sites.Fixes #9290