Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds CommonMark inline-comment patterns for test annotations, improves parsing to handle escaped/double-encoded JSON and normalize test.detectSteps/steps, swaps a dev test input from DITA to Markdown, replaces a raw HTTP example with a step-by-step walkthrough, and expands test coverage for comment-syntax variants. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used📓 Path-based instructions (3)**/*.{js,ts}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.test.js📄 CodeRabbit inference engine (AGENTS.md)
Files:
src/**/*.js📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (5)📓 Common learnings📚 Learning: 2025-11-25T23:59:45.298ZApplied to files:
📚 Learning: 2025-11-25T23:59:45.298ZApplied to files:
📚 Learning: 2025-11-25T23:59:45.298ZApplied to files:
📚 Learning: 2025-11-25T23:59:45.298ZApplied to files:
🧬 Code graph analysis (1)src/config.test.js (1)
🔇 Additional comments (7)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
📝 Documentation updates detected! New suggestion: Document CommonMark comment syntax quote style variations |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/config.js (1)
492-510: Log level string"warn"won’t ever be emitted by theloghelperIn the DOC_DETECTIVE env‑override error path you call:
log( config, "warn", `Invalid JSON in DOC_DETECTIVE environment variable: ${error.message}. Ignoring config overrides.` );but
logonly checks for"error","warning","info", and"debug". As written, this message will never be printed for anyconfig.logLevel.Change the level to
"warning"to make this path observable:- log( - config, - "warn", - `Invalid JSON in DOC_DETECTIVE environment variable: ${error.message}. Ignoring config overrides.` - ); + log( + config, + "warning", + `Invalid JSON in DOC_DETECTIVE environment variable: ${error.message}. Ignoring config overrides.` + );
🧹 Nitpick comments (4)
dev/doc-content.md (1)
1-19: markdownlint MD053 warnings are expected for[comment]: #usageThe
[comment]: # '...'lines are intentionally used as test metadata, not as real link reference definitions, so the MD053 “unused reference” warnings are false positives. Consider suppressing MD053 for this file or this specific[comment]label in your markdownlint config instead of changing the syntax.src/utils.js (2)
124-161: Gate escaped‑JSON unwrapping behind a failed direct JSON.parseThe new
looksLikeEscapedJsonpath nicely handles double‑quoted CommonMark JSON like{\\"testId\\": ...}, but it runs before a normalJSON.parse. That can mis-handle valid JSON that legitimately contains\"inside string values (e.g., JSON with embedded quotes), which previously parsed fine.Consider trying
JSON.parse(stringifiedObject)first and only applying the escaped‑JSON heuristic inside that catch block when parsing fails and the heuristic pattern matches. That keeps the new support for double‑encoded JSON without risking regressions for regular JSON content that happens to contain\".
581-627: detectSteps normalization and default steps array look correct; consider explicit test coverageConverting
"false"/"true"string values to booleans and ensuringtest.stepsis at least an empty array in thetestStartbranch aligns behavior with later checks likeif (typeof test.detectSteps !== "undefined" && !test.detectSteps)and the “remove tests with no steps” filter inparseTests.To lock this in, it would be useful to add at least one test where a test comment sets
"detectSteps": "false"(string) and assert that the resolved test hasdetectSteps === false(boolean).src/index.test.js (1)
889-1101: CommonMark JSON tests are strong; add one"detectSteps": "false"case to hit the new normalizationThe new
CommonMark Comment Syntax Testscover parentheses, single‑quote, double‑quote, and mixed‑quote JSON forms very well, and they assert bothtestIdand step shapes end‑to‑end throughdetectAndResolveTests.To exercise the
detectStepsstring‑to‑boolean normalization you just added inparseContent, consider adding or tweaking one test so the JSON uses"detectSteps": "false"(string) and then assert that the resolved test hasdetectSteps === false(boolean). That will guard against regressions in the normalization logic.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
dev/doc-content.md(1 hunks)dev/index.js(1 hunks)src/config.js(1 hunks)src/index.test.js(1 hunks)src/utils.js(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{js,ts}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts}: Use async/await for asynchronous operations
Prefer destructuring for function parameters
Use meaningful variable names that reflect Doc Detective terminology
Add JSDoc comments for complex functions
Files:
src/utils.jssrc/config.jssrc/index.test.jsdev/index.js
src/**/*.js
📄 CodeRabbit inference engine (AGENTS.md)
Use the built-in logging system with available log levels: debug, info, warn, error
Files:
src/utils.jssrc/config.jssrc/index.test.js
**/*.test.js
📄 CodeRabbit inference engine (AGENTS.md)
**/*.test.js: When possible, directly import and run functions rather than use extensive mocking and stubbing in tests
Mock external dependencies (file system, HTTP requests) in tests
Test both successful and error scenarios
Validate configuration handling thoroughly in tests
Use realistic test data that matches actual usage patterns
Use Mocha for unit tests
Use Chai for assertions
Files:
src/index.test.js
🧠 Learnings (6)
📚 Learning: 2025-11-25T23:59:45.298Z
Learnt from: CR
Repo: doc-detective/resolver PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T23:59:45.298Z
Learning: Applies to **/*.test.js : Use realistic test data that matches actual usage patterns
Applied to files:
src/utils.jssrc/index.test.js
📚 Learning: 2025-11-25T23:59:45.298Z
Learnt from: CR
Repo: doc-detective/resolver PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T23:59:45.298Z
Learning: Applies to **/*.test.js : Validate configuration handling thoroughly in tests
Applied to files:
src/utils.jssrc/index.test.jsdev/index.js
📚 Learning: 2025-11-25T23:59:45.298Z
Learnt from: CR
Repo: doc-detective/resolver PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T23:59:45.298Z
Learning: Applies to src/resolve.js : Follow existing regex patterns for markup detection when adding new features
Applied to files:
src/config.jssrc/index.test.js
📚 Learning: 2025-11-25T23:59:45.298Z
Learnt from: CR
Repo: doc-detective/resolver PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T23:59:45.298Z
Learning: Applies to **/*.{js,ts} : Add JSDoc comments for complex functions
Applied to files:
src/config.jssrc/index.test.js
📚 Learning: 2025-11-25T23:59:45.298Z
Learnt from: CR
Repo: doc-detective/resolver PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T23:59:45.298Z
Learning: Add comprehensive test coverage when adding new features
Applied to files:
src/index.test.js
📚 Learning: 2025-11-25T23:59:45.298Z
Learnt from: CR
Repo: doc-detective/resolver PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T23:59:45.298Z
Learning: Applies to **/*.test.js : Test both successful and error scenarios
Applied to files:
src/index.test.js
🪛 markdownlint-cli2 (0.18.1)
dev/doc-content.md
1-1: Link and image reference definitions should be needed
Unused link or image reference definition: "comment"
(MD053, link-image-reference-definitions)
5-5: Link and image reference definitions should be needed
Duplicate link or image reference definition: "comment"
(MD053, link-image-reference-definitions)
9-9: Link and image reference definitions should be needed
Duplicate link or image reference definition: "comment"
(MD053, link-image-reference-definitions)
10-10: Link and image reference definitions should be needed
Duplicate link or image reference definition: "comment"
(MD053, link-image-reference-definitions)
14-14: Link and image reference definitions should be needed
Duplicate link or image reference definition: "comment"
(MD053, link-image-reference-definitions)
18-18: Link and image reference definitions should be needed
Duplicate link or image reference definition: "comment"
(MD053, link-image-reference-definitions)
19-19: Link and image reference definitions should be needed
Duplicate link or image reference definition: "comment"
(MD053, link-image-reference-definitions)
🔇 Additional comments (4)
dev/index.js (1)
14-23: Markdown dev input path update matches new sample documentSwitching
inputtodev/doc-content.mdaligns the dev helper with the new Markdown-based sample content and extended CommonMark support; no issues from this change.src/config.js (1)
320-375: CommonMark[comment]: #inline statement regexes look consistent and robustThe added markdown
testStart,testEnd,ignore*, andsteppatterns for[comment]: # (...),'...', and"..."forms are consistent with existing inlineStatement conventions and correctly capture the payload into group 1, with the double‑quote variants using a safe(?:[^"\\]|\\.)*to handle escaped quotes. They line up with the new tests andparseObjectexpectations.src/index.test.js (2)
1105-1301: YAML-based CommonMark tests nicely cover JSON/YAML interchangeThe
CommonMark Comment Syntax with YAML Testsblock drives realistic YAML payloads through all three[comment]: # (/'/")variants, and the assertions ontestId,goTo,type, andfindsteps confirm that both YAML parsing and the markdown patterns are wired up correctly. No issues from these changes.
1304-1411: XML attribute + dot-notation CommonMark tests give excellent coverage of parseXmlAttributesThe XML-attribute and XML-dot CommonMark tests thoroughly exercise
parseXmlAttributes, including booleans (detectSteps=false), numbers (wait=500), and nestedhttpRequest.*properties. The expectations on nestedhttpRequest.url/method/request.bodyobjects match the intended behavior and validate the new markdown patterns end‑to‑end.
There was a problem hiding this comment.
Pull request overview
This PR enhances the application's ability to parse test definitions from CommonMark-style markdown comments by supporting multiple quote syntax variations (parentheses, single quotes, and double quotes). It also improves JSON parsing for escaped/double-encoded JSON content and normalizes the detectSteps field to ensure consistent boolean values.
Key Changes
- Added regex patterns to recognize CommonMark comment syntax with three different quote styles:
[comment]: # (...),[comment]: # '...', and[comment]: # "..." - Implemented fallback JSON parsing logic to handle escaped and double-encoded JSON strings
- Added normalization for
detectStepsfield to convert string values "true"/"false" to boolean types
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/config.js | Added regex patterns for CommonMark comment variations (parentheses, single quotes, double quotes) across all statement types; changed log level from "warn" to "warning" |
| src/config.test.js | Updated test assertion to match log level change from "warn" to "warning" |
| src/utils.js | Added escaped/double-encoded JSON parsing fallback logic; added detectSteps string-to-boolean normalization; updated comment for clarity |
| src/index.test.js | Added comprehensive test suites covering all CommonMark syntax variations with JSON, YAML, and XML attribute content formats |
| dev/index.js | Changed test input file from DITA to Markdown format |
| dev/doc-content.md | Replaced HTTP example with interactive test walkthrough using CommonMark comment syntax |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Enhance the application by supporting more variations of CommonMark syntax for test comments and normalizing the
detectStepsfield. Improve the parsing of double-quoted JSON to handle escaped characters more effectively.Summary by CodeRabbit
Documentation
New Features
Bug Fixes
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.