fix(verify-cv-facts): stop flagging ordinary prose as tool claims - #3643
Conversation
Scott-Emberson
left a comment
There was a problem hiding this comment.
Reviewed at head 38c792fc2a. On a source-of-truth surface like the fact gate, the risk with a false-positive fix is over-correcting into silence, and this one does not. The owned test asserts both directions: the five prose cases that were being wrongly flagged (a gerund or abstract noun after "using") now come back with zero tool claims, but a genuine unbacked tool claim still blocks in both its lowercase form ("using kubernetes and google cloud") and its Title-Cased form ("Kubernetes and Terraform" with no source), and a source-backed lowercase claim ("kubernetes and n8n") passes. The fix only adds a source-backed exemption to isLikelyTool plus a narrow prose-suffix drop that fires only for fragments that are neither Title-Cased/versioned nor source-backed, and the "still blocks" cases guard against that suffix drop silencing real fabrication. The pre-existing "explicit lowercase tool claims fail closed without a whitelist entry" assertion still passes.
Owned test verified sound, the no-fabrication gate still catches real unbacked claims. Holding formal approval until the branch is finalized, nothing blocking from the tests/ side.
Scott-Emberson
left a comment
There was a problem hiding this comment.
Approving from the tests/ side. The owned test is verified sound at this head (evidence in my review note above), CI is green, and the branch is mergeable. Good to go from our end. As always this routes the code-owner review, it does not merge.
…reer-ops-hq#3639) factClaims()'s 'tool' extraction pattern was too broad: everything after a trigger word (using/built with/worked with/technologies:/tech stack:) up to the next punctuation was captured, split on commas/and/with/in, and any resulting fragment of <=3 words not in a small stoplist was treated as an unverified tool claim. That flagged ordinary prose like "improving on-time submission", "recurring HR", "diagnosing", "feedback", and "efficiency" as fabricated tools, forcing repeated rewrites of truthful CV/cover-letter text. Fix combines two of the issue's three suggested directions rather than picking one exclusively: - A fragment that looks tool-shaped (Title-Cased, or carries a digit/version token like "n8n" or "GPT-4") is accepted outright — this is direction 3's positive signal, and it also protects real lowercase tool names that are already listed in a source file (verified with a new test). - A fragment that is neither tool-shaped nor source-backed is now dropped only when it carries a common English gerund/abstract-noun suffix (-ing/-tion/-sion/-ment/-ency/-ance/-ery/-ity/-ness), the shape shared by every concrete false positive in the issue. This targets the actual failure class instead of hand-listing more stoplist words one at a time (direction 2's whack-a-mole problem). - Anything else (e.g. a lowercase, non-prose-shaped fragment like "kubernetes" or "google cloud" with no source backing) falls through to the pre-existing fail-closed behavior unchanged, so a fabricated tool cannot evade detection just by typing it in lowercase. Added tests/nonmetric-fact-gate.test.mjs coverage for: every concrete false-positive example from career-ops-hq#3639, a source-backed lowercase tool passing cleanly, a lowercase fabricated tool still blocking, and a Title-Cased fabricated tool still blocking. Fixes career-ops-hq#3639 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
38c792f to
6acc7fd
Compare
Scott-Emberson
left a comment
There was a problem hiding this comment.
Re-reviewed at head 6acc7fd (moved past the earlier approval on a large merge). The subject test tests/nonmetric-fact-gate.test.mjs did genuinely change, but the change is additive and strengthening, and it is not made redundant by the similarly-titled #3639 that landed in main: it adds the gerund and abstract-noun false-positive cases, keeps the lowercase and Title-Case fabrication-still-blocks guards, and adds a distinct delegatedAuthorshipClaims block for vendor and contractor relative-clause escalation, importing an export that is this PR's own subject. No existing assertion was removed or loosened, the embedded-substring boundary test and the prior cases are all retained.
The substance is verify-cv-facts.mjs, a source-of-truth surface outside our owned paths, so this is a test-axis review: the owned test is sound and the anti-fabrication coverage is extended, the scope and merge of the change are the maintainer's call. Nothing blocking from the tests/ side.
luochen211
left a comment
There was a problem hiding this comment.
The suffix heuristic opens a fail-open path for real lowercase technology names. PROSE_SUFFIX_RE drops any unbacked lowercase fragment ending in ing|tion|sion|ment|ency|ance|ery|ity|ness; direct calls such as factClaims('Built this using spring.'), ... unity., and ... processing. all return no tool claim. Those are real technologies, so a fabricated lowercase claim now bypasses the gate instead of blocking.
The new tests prove only names outside the suffix set (kubernetes, google cloud). Please avoid using a blanket English suffix as the deciding signal, or add an evidence-backed technology recognition path, and add regressions for lowercase product names that collide with these suffixes.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe tool-claim filter now rejects prose-shaped fragments, preserves blocking for unsupported tools, and permits lowercase technology claims when normalized source text contains exact matches. Regression tests cover these cases. ChangesTool claim validation
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to Tool-claim extraction now avoids prose false positives while retaining unsupported-tool blocking, with regression coverage for accepted and rejected patterns. No current merge-blocking risk remains. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Linked Issues checkExplanation The changes satisfy issue Full details: Out of Scope Changes checkExplanation The changes remain within issue ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Scott-Emberson
left a comment
There was a problem hiding this comment.
Re-reviewed at head fc22a4d. The rebase strengthened the fact gate in both directions: lowercase tech names (spring, unity, processing) stay fail-closed and block, and source evidence now overrides an exact prose-word collision so there is no over-block. The named-vendor escalation-blocks case and the source-of-truth behavior are intact. Nothing earlier was weakened, and the anti-fabrication direction is hardened.
The substance is verify-cv-facts.mjs, a source-of-truth surface outside our owned paths, so this is a test-axis review: the owned test is sound and the coverage is extended, the scope and merge are the maintainer's call. The branch is behind main, so a rebase before merge would help, but nothing is blocking from the tests/ side.
luochen211
left a comment
There was a problem hiding this comment.
Re-reviewed current head fc22a4d. The blanket suffix exemption is gone; unbacked lowercase Spring, Unity, and Processing claims remain fail-closed, while the prose false positives are handled by bounded exact words and source-backed collisions still pass. Focused nonmetric fact-gate suite passes locally (30/30). My prior blocking finding is resolved.
|
Another solid one, @Schlaflied. Keeping the stoplist to exact words instead of suffixes, so Spring, Unity and Processing stay fail-closed, is the distinction that makes this safe, and the source-backed exemption means a real lowercase |
…l claim `isLikelyTool()` accepted any short lowercase fragment by default, so an ordinary "using" sentence was asserted as a technology name and `assertFacts()` blocked the render of a document that claimed nothing false. career-ops-hq#3643 added a shape test and a source-backed exemption, but both only add accept paths, which left `TOOL_PROSE_WORDS` as the only thing standing between ordinary prose and a tool claim: a list holding the words that happened to appear in career-ops-hq#3639. Drop a fragment whose every word already occurs in the source. That is the document's own vocabulary reworded, which is exactly what tailoring does to a "using" sentence. A name the source never mentions is untouched, so a lowercase fabrication still blocks. Widen the existing leading-article drop to the rest of the determiner class, so "using that campaign" is not read as a product name whatever the source happens to contain. Closes career-ops-hq#4004
Summary
factClaims()'s'tool'extraction inverify-cv-facts.mjscaptured everything after a trigger word (using,built with,worked with,technologies:,tech stack:) up to the next punctuation, split it on commas/and/with/in, and treated any resulting fragment of ≤3 words not inTOOL_PROSE_WORDSas an unverified tool claim. That flagged ordinary prose — "improving on-time submission", "recurring HR", "diagnosing", "feedback", "efficiency" — as fabricated tools, forcing rewrites of completely truthful CV/cover-letter text to dodge the trigger grammar.Direction chosen (issue asked for a justification since it's ambiguous)
The issue proposed three directions. This PR combines (1)/(3) and (2) rather than picking one exclusively:
cv.md(e.g.kubernetes) isn't penalized for casing.-ing/-tion/-sion/-ment/-ency/-ance/-ery/-ity/-ness) — the shape every concrete false positive in the issue shared. This targets the actual failure class instead of adding one stoplist word at a time.kubernetesorgoogle cloud) falls through to the pre-existing fail-closed behavior, unchanged — so a fabricated tool can't evade detection just by lowercasing it.One item from the issue's table — the "byte-lossless markdown-to-database round-trip" example — is a distinct root cause (an exact-string mismatch against
cv.md'smarkdown↔DBphrasing) rather than the extraction-breadth problem this issue is about, and is out of scope here; the issue itself flags it as "not a fabrication" separately from the extraction bug.Changes
verify-cv-facts.mjs: addedlooksToolShaped()andPROSE_SUFFIX_RE;isLikelyTool()now takes an optionalsourceNormalizedparam so a lowercase-but-genuine tool can be recognized against the source;factClaims()threads it through fromverifyFacts().tests/nonmetric-fact-gate.test.mjs: added coverage for every concrete false-positive example from verify-cv-facts.mjs: 'using X' tool-claim extraction flags ordinary prose as unverified tool claims #3639, a source-backed lowercase tool passing cleanly, a lowercase fabricated tool still blocking, and a Title-Cased fabricated tool still blocking.Test plan
node test-all.mjs— full suite: 7694 passed, 0 failed, 13 warnings (pre-existing, unrelated)toolclaimsBuilt with Kubernetes and Terraform.andShipped it using kubernetes and google cloud.(both fabricated, no source backing) still get flagged/blockedFixes #3639
🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests