fix(detector): add --source-mode to the bundled detect.js - #340
Conversation
PR Summary by QodoAdd source-mode parity to the bundled detector CLI
AI Description
Diagram
High-Level Assessment
Files changed (5)
|
Code Review by Qodo
1.
|
e91e5a1 to
0948bbc
Compare
skills/ai-writing-detector/scripts/detect.js supports only --file and --context, so the published plugin cannot reach rendered-Markdown scoring. On a Markdown file with YAML frontmatter the root CLI reports Clean while the bundled script flags all three frontmatter lines, and passing the flag throws an uncaught "unknown argument" stack trace instead of the root CLI's exit-2 message. Port the one flag, as scoped in conorbronsdon#244, and close the parity gaps in the two paths the bundled script shares with the root CLI: - add --source-mode <plain|rendered-markdown>, validated, passed through as sourceMode to analyzeText; - accept the marketing and personal contexts the root CLI and the shared detector already support, so equivalent invocations agree instead of the bundled parser rejecting two valid values; - mirror the root CLI's empty-result normalization, so blank input reports the selected context and source mode rather than an empty stats object; - replace the throws on a bad argument with a usage message and exit 2, matching the root CLI (including -h/--help); - add scripts/detect-parity.test.js, which spawns both entry points on the same file, compares the JSON across every supported context and both source modes, covers empty and whitespace-only input, asserts rendered-markdown returns Clean where plain mode scores the frontmatter, and pins the exit-2 error cases without a stack trace; - document the flag in the sub-skill SKILL.md next to --context; - register the new suite in scripts/run-tests.js. detect.js keeps its --file/--context interface; the root CLI keeps its positional path. Only the flag is ported. Refs conorbronsdon#244
0948bbc to
6c06cc6
Compare
|
Both review findings are addressed in
The review summary still lists one open requirement gap; that reflects The new suite is registered in |
conorbronsdon
left a comment
There was a problem hiding this comment.
Thank you, @ayduanzf-hub — this is a careful follow-up, especially the all-context and blank-input parity coverage. I restored the executable bit on detect.js in 68b59f5; no source bytes changed. The full suite, plugin validator, sync checks, mutation control, and all hosted checks are green. Approved.
One contributor-policy note for next time: #244 was later labeled good first issue. We reserve those issues for one first contribution per person, including while that first PR awaits review. I'm happy to land this one because the issue was not originally presented that way, but please choose a help wanted issue without good first issue for your next contribution. A few unclaimed options:
- #85 — the documentation or conversational corpus leg
- #94 — the Vale package exploration; coordinate the repository and coverage decision first
- #238 — quoted-material masking; comment first so we can resolve the listed design choices
Thanks again for both contributions.
Summary
skills/ai-writing-detector/scripts/detect.jssupported only--fileand--context, so the published plugin could not reach rendered-Markdown scoring. On a Markdown file with YAML frontmatter the root CLI returns[0, "Clean", []]while the bundled script flags all three frontmatter lines, and passing the flag threw an uncaughtunknown argumentstack trace instead of the root CLI's exit-2 message.Ported the one flag, as scoped in #244.
detect.jskeeps its--file/--contextinterface and the root CLI keeps its positional path; the two are not unified.--source-mode <plain|rendered-markdown>, validated and passed through assourceModetoanalyzeText. No detector change was needed: the masking already lives indetector/patterns.js.marketingandpersonalcontexts thatbin/avoid-ai-writing.jsandVALID_CONTEXT_MODESinpatterns.jsalready support. The bundled parser rejected both, so an invocation the root CLI accepts exited 2 here.stats: {}.throw new Error(...)calls with a usage message andprocess.exit(2), matching the root CLI, and add-h/--help.scripts/detect-parity.test.js, registered inscripts/run-tests.js. It spawns both entry points on the same file, compares their JSON for every supported context and both source modes, covers empty and whitespace-only input, assertsrendered-markdownreturns Clean where plain mode scores the frontmatter, and pins the exit-2 error cases (usage on stderr, nothing on stdout, no stack trace).skills/ai-writing-detector/SKILL.mdnext to--context.## [Unreleased]changelog entry, since this changes the published plugin.On the same fixture through the bundled script:
plain(default)rendered-markdownAll four contexts and both source modes produce byte-identical JSON through both entry points, including on blank input.
Neither sync script generates
detect.js—sync-plugin-skill.shcopies onlypatterns.js,validate.js, andCATEGORIES.mdinto the bundled directory — so the drift this issue reports cannot recur through them. Running both leaves the tree clean apart from the five files here.Checklist
npm testpasses (engine fixtures +CATEGORIES.mdcontract check)type: it's documented indetector/CATEGORIES.mdand has a fixture indetector/patterns.test.js(a true positive and a must-not-fire case) — not applicable, no detector type addeddetector/CATEGORIES.md— not applicableCHANGELOG.mdentry for a user-facing change, or this PR is exempt under the changelog policy###inreferences/patterns.md: the**NN pattern categories**bullet inREADME.mdand the quoted count inCLAUDE.mdmatchscripts/check-pattern-count.sh— not applicable, no category changeFixes #244