Skip to content

Standardize maxVariation logic and rename difference calculation#400

Merged
hawkeyexl merged 7 commits intomainfrom
maxvariation-standardization
Dec 3, 2025
Merged

Standardize maxVariation logic and rename difference calculation#400
hawkeyexl merged 7 commits intomainfrom
maxvariation-standardization

Conversation

@hawkeyexl
Copy link
Copy Markdown
Contributor

@hawkeyexl hawkeyexl commented Dec 2, 2025

Standardize the logic and logging for maxVariation to use fractional differences instead of percentages. Rename calculatePercentageDifference() to calculateFractionalDifference() and update related documentation accordingly.

Summary by CodeRabbit

  • Documentation
    • Expanded architecture, workflows, validation, error-handling, OpenAPI, testing, and CI/CD guidance; clarified maxVariation semantics as fractional (0–1) and added examples and standardized return shapes.
  • Refactor
    • Switched difference calculations and threshold checks from percentage to fractional values across the system; updated user-facing logs and messages to report fractional differences with adjusted formatting.
  • Bug Fixes
    • Added explicit validation for steps and improved error/logging and cleanup behavior.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Dec 2, 2025

Note

Other AI code review bot(s) detected

CodeRabbit 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.

Walkthrough

Converts difference calculations from percentage (0–100) to fractional (0–1), renames calculatePercentageDifferencecalculateFractionalDifference, updates its implementation to return a numeric fraction (Levenshtein for text), and updates tests, logs, threshold checks, image-diff math, and documentation/CI wording to use fractional semantics.

Changes

Cohort / File(s) Summary
Core utility
src/utils.js
Renamed export/function calculatePercentageDifferencecalculateFractionalDifference; implementation now returns a numeric fractional difference (0–1) using Levenshtein distance; JSDoc and return shape updated; empty-string guard added.
Text & Shell tests
src/tests/httpRequest.js, src/tests/runShell.js
Imports switched to calculateFractionalDifference; local vars/logs renamed from percentDifffractionalDiff; threshold checks changed from percentDiff > maxVariation * 100 to fractionalDiff > maxVariation; messages and formatting updated.
Screenshot/image test
src/tests/saveScreenshot.js
Image diff computed as numDiffPixels / (width * height) (fractional) instead of percentage; variables, checks, and messages updated to fractional representation; control flow preserved.
Documentation & CI
AGENTS.md, src/openapi.js, src/expressions.js, src/tests/...
Expanded documentation across architecture, workflows, validation, error handling, testing patterns, CI/CD (post-publish and resolver workflows); added or referenced src/openapi.js, src/expressions.js, and src/tests/ structure; changed wording/examples from percentage to fractional semantics.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Check src/utils.js edge cases (both-empty strings → 0, bounds clamped to [0,1]).
  • Verify all callers and test fixtures interpret maxVariation as fractional (0–1).
  • Confirm image-diff math, formatting (toFixed), and any persisted artifacts/assertions updated to fractional values.
  • Review documentation/CI wording for consistency with the new fractional semantics.

Poem

🐇 I measured small hops, not a cent,
From hundredfold steps to fractions went,
Renamed my paws, made logs quite neat,
Tiny decimals—soft and fleet. 🥕

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 42.86% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main changes: standardizing maxVariation logic to use fractional differences and renaming the core calculation function.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch maxvariation-standardization

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 01defc1 and 9fac9d1.

📒 Files selected for processing (1)
  • AGENTS.md (5 hunks)
🧰 Additional context used
🧠 Learnings (17)
📓 Common learnings
Learnt from: CR
Repo: doc-detective/core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T19:31:27.066Z
Learning: Applies to src/tests/**/*.js : Treat `maxVariation` percentage values as decimals (0.1 = 10%) but multiply by 100 when performing percentage-based comparisons
📚 Learning: 2025-11-26T19:31:27.066Z
Learnt from: CR
Repo: doc-detective/core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T19:31:27.066Z
Learning: Applies to src/tests.js : Core test runner `src/tests.js` orchestrates Appium/WebDriver sessions and manages spec → test → context → step hierarchy (600+ LOC orchestrator)

Applied to files:

  • AGENTS.md
📚 Learning: 2025-11-26T19:31:27.066Z
Learnt from: CR
Repo: doc-detective/core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T19:31:27.066Z
Learning: Input resolution via `doc-detective-resolver` detects tests from docs/specs and resolves to executable format as first step in test execution flow

Applied to files:

  • AGENTS.md
📚 Learning: 2025-11-26T19:31:27.066Z
Learnt from: CR
Repo: doc-detective/core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T19:31:27.066Z
Learning: Tests follow nested hierarchy: spec (file) → test → context (browser/platform combo) → step (action), with contexts running serially and skipping if unsupported

Applied to files:

  • AGENTS.md
📚 Learning: 2025-11-26T19:31:27.066Z
Learnt from: CR
Repo: doc-detective/core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T19:31:27.066Z
Learning: Test & Publish workflow runs cross-platform tests (Ubuntu/Windows/macOS × Node 18/20/22/24) before publishing; triggers downstream `doc-detective` update via repository dispatch

Applied to files:

  • AGENTS.md
📚 Learning: 2025-11-26T19:31:27.066Z
Learnt from: CR
Repo: doc-detective/core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T19:31:27.066Z
Learning: Each step type has dedicated handler module in `src/tests/` (e.g., `httpRequest.js`, `runShell.js`) implementing specific action logic

Applied to files:

  • AGENTS.md
📚 Learning: 2025-11-26T19:31:27.066Z
Learnt from: CR
Repo: doc-detective/core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T19:31:27.066Z
Learning: Applies to src/index.js : Entry point `src/index.js` must expose `runTests()` function as the primary API for test execution

Applied to files:

  • AGENTS.md
📚 Learning: 2025-11-26T19:31:27.066Z
Learnt from: CR
Repo: doc-detective/core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T19:31:27.066Z
Learning: Run local test suite with `npm test` (mocha), use `node dev` for development/manual testing, run `npm run depcheck` to check unused dependencies

Applied to files:

  • AGENTS.md
📚 Learning: 2025-11-26T19:31:27.066Z
Learnt from: CR
Repo: doc-detective/core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T19:31:27.066Z
Learning: Auto Dev Release workflow skips on `[skip ci]` commits, release commits, or docs-only changes; increments dev version format `X.Y.Z-dev.N`

Applied to files:

  • AGENTS.md
📚 Learning: 2025-11-26T19:31:27.066Z
Learnt from: CR
Repo: doc-detective/core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T19:31:27.066Z
Learning: Update Resolver workflow syncs dependencies: installs specified resolver version + matching `doc-detective-common` version, runs tests, bumps patch version, creates release with aggregated PR notes and resolver changelog

Applied to files:

  • AGENTS.md
📚 Learning: 2025-11-26T19:31:27.066Z
Learnt from: CR
Repo: doc-detective/core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T19:31:27.066Z
Learning: Applies to src/tests.js : Register new step types in `src/tests.js`: add action to `driverActions` array if requires browser, add case in `runStep()` switch statement

Applied to files:

  • AGENTS.md
📚 Learning: 2025-11-26T19:31:27.066Z
Learnt from: CR
Repo: doc-detective/core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T19:31:27.066Z
Learning: Applies to scripts/postinstall.js : Post-install script (`scripts/postinstall.js`) must auto-download Chrome/Firefox/ChromeDriver to `browser-snapshots/` directory matching current platform

Applied to files:

  • AGENTS.md
📚 Learning: 2025-11-26T19:31:27.066Z
Learnt from: CR
Repo: doc-detective/core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T19:31:27.066Z
Learning: Applies to src/tests/*.js : Create new step type handlers in `src/tests/[actionName].js` exporting async functions that validate schema, resolve to object, set defaults, and return `{ status: 'PASS'|'FAIL'|'WARNING', description: string, outputs: {} }`

Applied to files:

  • AGENTS.md
📚 Learning: 2025-11-26T19:31:27.066Z
Learnt from: CR
Repo: doc-detective/core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T19:31:27.066Z
Learning: Applies to src/tests.js : Configure Appium drivers (`chromium`, `gecko`, `safari` on macOS) with default 10-minute timeout via `newCommandTimeout: 600`

Applied to files:

  • AGENTS.md
📚 Learning: 2025-11-26T19:31:27.066Z
Learnt from: CR
Repo: doc-detective/core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T19:31:27.066Z
Learning: Applies to src/tests/**/*.js : Set step variables via `step.variables = { MY_VAR: '$$response.body.token' }` to store runtime values as environment variables for subsequent steps

Applied to files:

  • AGENTS.md
📚 Learning: 2025-11-26T19:31:27.066Z
Learnt from: CR
Repo: doc-detective/core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T19:31:27.066Z
Learning: Applies to src/tests/**/*.js : Always handle driver cleanup in try/finally blocks to ensure WebDriver sessions are properly terminated

Applied to files:

  • AGENTS.md
📚 Learning: 2025-11-26T19:31:27.066Z
Learnt from: CR
Repo: doc-detective/core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T19:31:27.066Z
Learning: Applies to src/config.js : Validate configuration objects via `doc-detective-common` schemas using `validate({ schemaKey: 'config_v3', object })`

Applied to files:

  • AGENTS.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
  • GitHub Check: test (ubuntu-latest, 24)
  • GitHub Check: test (windows-latest, 22)
  • GitHub Check: test (windows-latest, 24)
  • GitHub Check: test (windows-latest, 20)
  • GitHub Check: test (macos-latest, 20)
  • GitHub Check: test (macos-latest, 22)
  • GitHub Check: test (ubuntu-latest, 20)
  • GitHub Check: test (ubuntu-latest, 22)
🔇 Additional comments (3)
AGENTS.md (3)

88-90: Markdown linting issue resolved correctly.

The fenced code block now properly specifies the text language tag, satisfying MD040 requirements. ✓


130-131: Clear and accurate fractional variation documentation.

The documentation at line 130 correctly conveys the semantic shift: maxVariation now uses decimal fractions (0.1 = 10%) with comparisons operating directly on fractions rather than percentages. The example and explanation are concise and align with the PR objectives. The reference in line 131 to "aboveVariation" behavior is consistent with this fractional interpretation.


1-154: Comprehensive documentation expansion is well-structured and accurate.

The large-scale rewrite of AGENTS.md provides thorough coverage of architecture, CI/CD workflows, version management, step type conventions, expression system, OpenAPI integration, error handling, and common pitfalls. Documentation is clear, consistent, and appropriately detailed for a developer-facing guide. No contradictions or misleading statements detected.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (2)
src/tests/saveScreenshot.js (1)

289-289: Fractional image diff logic is consistent; consider clarifying messages

The switch to fractionalDiff = numDiffPixels / (width * height) and comparing directly against maxVariation aligns with the new fractional semantics and the defaults (0.05 → 5% tolerance). Logging with fractionalDiff.toFixed(2) is also consistent with the other step types.

One minor clarity point: the warning message still refers to “command output content”, which is confusing in a screenshot step. Consider rewording to something like:

-        result.description += ` The difference between the existing file content and command output content (${fractionalDiff.toFixed(
+        result.description += ` The difference between the existing screenshot and new screenshot (${fractionalDiff.toFixed(
           2
         )}) is greater than the max accepted variation (${
           step.screenshot.maxVariation
         }).`;

Also applies to: 340-345, 347-357, 360-362

src/tests/httpRequest.js (1)

7-7: HTTP fixture diffing correctly migrated to fractional differences

Importing calculateFractionalDifference and using fractionalDiff > step.httpRequest.maxVariation for saved-response comparisons brings this step in line with the new fractional maxVariation semantics. With the default maxVariation: 0, any change in the serialized JSON will still cause a failure, which matches the previous strict behavior. The small doc/formatting tweaks in fieldExistsAtPath are also fine.

If you want the failure message to be less shell-specific, you could optionally adjust the wording:

-        result.description += ` The difference between the existing file content and command output content (${fractionalDiff.toFixed(
+        result.description += ` The difference between the existing saved response and current response (${fractionalDiff.toFixed(
           2
         )}) is greater than the max accepted variation (${
           step.httpRequest.maxVariation
         }).`;

Also applies to: 456-462, 463-477, 491-535

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b691a29 and 195abe4.

📒 Files selected for processing (5)
  • AGENTS.md (1 hunks)
  • src/tests/httpRequest.js (7 hunks)
  • src/tests/runShell.js (2 hunks)
  • src/tests/saveScreenshot.js (2 hunks)
  • src/utils.js (2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
src/tests/*.js

📄 CodeRabbit inference engine (AGENTS.md)

Create new step type handlers in src/tests/[actionName].js exporting async functions that validate schema, resolve to object, set defaults, and return { status: 'PASS'|'FAIL'|'WARNING', description: string, outputs: {} }

Files:

  • src/tests/runShell.js
  • src/tests/saveScreenshot.js
  • src/tests/httpRequest.js
src/**/*.js

📄 CodeRabbit inference engine (AGENTS.md)

src/**/*.js: Use log(config, level, message) for logging where level is 'debug'|'info'|'warning'|'error', passing config object as first parameter
Use getAvailableApps() to detect installed browsers rather than hardcoding browser paths, accounting for platform-specific locations

Files:

  • src/tests/runShell.js
  • src/tests/saveScreenshot.js
  • src/utils.js
  • src/tests/httpRequest.js
src/tests/**/*.js

📄 CodeRabbit inference engine (AGENTS.md)

src/tests/**/*.js: Always handle driver cleanup in try/finally blocks to ensure WebDriver sessions are properly terminated
Set step variables via step.variables = { MY_VAR: '$$response.body.token' } to store runtime values as environment variables for subsequent steps
Use setViewportSize() to set inner viewport dimensions, which calculates delta from window size rather than setting window size directly
Treat maxVariation percentage values as decimals (0.1 = 10%) but multiply by 100 when performing percentage-based comparisons
File overwrite modes support three values: 'false' (never overwrite), 'true' (always overwrite), 'aboveVariation' (overwrite only if content differs > maxVariation)

Files:

  • src/tests/runShell.js
  • src/tests/saveScreenshot.js
  • src/tests/httpRequest.js
src/tests/httpRequest.js

📄 CodeRabbit inference engine (AGENTS.md)

Reference OpenAPI operations by operationId (e.g., step.httpRequest.openApi.operationId = 'getUserById') rather than path/method combinations

Files:

  • src/tests/httpRequest.js
🧠 Learnings (7)
📓 Common learnings
Learnt from: CR
Repo: doc-detective/core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T19:31:27.066Z
Learning: Applies to src/tests/**/*.js : Treat `maxVariation` percentage values as decimals (0.1 = 10%) but multiply by 100 when performing percentage-based comparisons
📚 Learning: 2025-11-26T19:31:27.066Z
Learnt from: CR
Repo: doc-detective/core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T19:31:27.066Z
Learning: Applies to src/tests/**/*.js : Treat `maxVariation` percentage values as decimals (0.1 = 10%) but multiply by 100 when performing percentage-based comparisons

Applied to files:

  • src/tests/runShell.js
  • src/tests/saveScreenshot.js
  • AGENTS.md
  • src/utils.js
  • src/tests/httpRequest.js
📚 Learning: 2025-11-26T19:31:27.066Z
Learnt from: CR
Repo: doc-detective/core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T19:31:27.066Z
Learning: Applies to src/tests/**/*.js : File overwrite modes support three values: 'false' (never overwrite), 'true' (always overwrite), 'aboveVariation' (overwrite only if content differs > maxVariation)

Applied to files:

  • src/tests/runShell.js
  • src/tests/saveScreenshot.js
  • AGENTS.md
  • src/tests/httpRequest.js
📚 Learning: 2025-11-26T19:31:27.066Z
Learnt from: CR
Repo: doc-detective/core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T19:31:27.066Z
Learning: Applies to src/tests/**/*.js : Use `setViewportSize()` to set inner viewport dimensions, which calculates delta from window size rather than setting window size directly

Applied to files:

  • src/tests/runShell.js
  • src/tests/saveScreenshot.js
  • AGENTS.md
📚 Learning: 2025-11-26T19:31:27.066Z
Learnt from: CR
Repo: doc-detective/core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T19:31:27.066Z
Learning: Applies to src/**/*.js : Use `getAvailableApps()` to detect installed browsers rather than hardcoding browser paths, accounting for platform-specific locations

Applied to files:

  • AGENTS.md
📚 Learning: 2025-11-26T19:31:27.066Z
Learnt from: CR
Repo: doc-detective/core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T19:31:27.066Z
Learning: Applies to src/tests/httpRequest.js : Reference OpenAPI operations by `operationId` (e.g., `step.httpRequest.openApi.operationId = 'getUserById'`) rather than path/method combinations

Applied to files:

  • src/tests/httpRequest.js
📚 Learning: 2025-11-26T19:31:27.066Z
Learnt from: CR
Repo: doc-detective/core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T19:31:27.066Z
Learning: Applies to src/openapi.js : In OpenAPI integration, extract request/response examples from spec, validate payloads via AJV against schemas, and support mock responses by setting `step.httpRequest.openApi.mockResponse = true`

Applied to files:

  • src/tests/httpRequest.js
🧬 Code graph analysis (3)
src/tests/runShell.js (2)
src/tests/httpRequest.js (2)
  • fractionalDiff (457-460)
  • existingFile (454-454)
src/utils.js (1)
  • fractionalDiff (279-279)
src/tests/saveScreenshot.js (3)
src/tests/httpRequest.js (2)
  • fractionalDiff (457-460)
  • result (12-12)
src/tests/runShell.js (2)
  • fractionalDiff (138-141)
  • result (15-25)
src/utils.js (1)
  • fractionalDiff (279-279)
src/utils.js (3)
src/tests/httpRequest.js (1)
  • fractionalDiff (457-460)
src/tests/runShell.js (1)
  • fractionalDiff (138-141)
src/tests/saveScreenshot.js (1)
  • fractionalDiff (289-289)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
  • GitHub Check: test (macos-latest, 24)
  • GitHub Check: test (macos-latest, 20)
  • GitHub Check: test (windows-latest, 22)
  • GitHub Check: test (macos-latest, 22)
  • GitHub Check: test (windows-latest, 24)
  • GitHub Check: test (windows-latest, 20)
  • GitHub Check: test (ubuntu-latest, 20)
  • GitHub Check: test (ubuntu-latest, 24)
  • GitHub Check: test (ubuntu-latest, 22)
🔇 Additional comments (3)
AGENTS.md (1)

112-112: Fractional variation doc matches new semantics

The updated maxVariation description (fractional 0–1, e.g., 0.1 = 10%) correctly aligns with the new comparison logic used in the tests and utilities.

src/tests/runShell.js (1)

5-6: Shell output diff now correctly uses fractional semantics

Using calculateFractionalDifference on the actual file contents and comparing fractionalDiff > step.runShell.maxVariation brings this step in line with the new fractional maxVariation contract. Overwrite behavior for "false", "true", and "aboveVariation" remains intact.

No functional issues spotted here; any further tweaks (e.g., message wording) would be purely cosmetic.

Also applies to: 137-153

src/utils.js (1)

15-15: New calculateFractionalDifference implementation is correct

Levenshtein distance normalized by max string length with proper zero-length guard yields accurate 0–1 fractional differences. All call sites in runShell.js, httpRequest.js, and saveScreenshot.js correctly compare fractional differences directly against maxVariation thresholds as decimal values (0.1 for 10% tolerance). No stale references to the previous calculatePercentageDifference API remain in the codebase.

@promptless
Copy link
Copy Markdown

promptless bot commented Dec 2, 2025

📝 Documentation updates detected!

New suggestion: Update maxVariation documentation to use fractional terminology

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
src/tests/httpRequest.js (1)

305-319: LGTM! Formatting improvements enhance readability.

The whitespace additions in the required fields validation block (lines 305-319) and the fieldExistsAtPath function (lines 496-537) improve code organization without any functional changes.

Also applies to: 496-537

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 195abe4 and 1763a44.

📒 Files selected for processing (3)
  • src/tests/httpRequest.js (7 hunks)
  • src/tests/runShell.js (2 hunks)
  • src/tests/saveScreenshot.js (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/tests/runShell.js
🧰 Additional context used
📓 Path-based instructions (4)
src/tests/*.js

📄 CodeRabbit inference engine (AGENTS.md)

Create new step type handlers in src/tests/[actionName].js exporting async functions that validate schema, resolve to object, set defaults, and return { status: 'PASS'|'FAIL'|'WARNING', description: string, outputs: {} }

Files:

  • src/tests/httpRequest.js
  • src/tests/saveScreenshot.js
src/**/*.js

📄 CodeRabbit inference engine (AGENTS.md)

src/**/*.js: Use log(config, level, message) for logging where level is 'debug'|'info'|'warning'|'error', passing config object as first parameter
Use getAvailableApps() to detect installed browsers rather than hardcoding browser paths, accounting for platform-specific locations

Files:

  • src/tests/httpRequest.js
  • src/tests/saveScreenshot.js
src/tests/**/*.js

📄 CodeRabbit inference engine (AGENTS.md)

src/tests/**/*.js: Always handle driver cleanup in try/finally blocks to ensure WebDriver sessions are properly terminated
Set step variables via step.variables = { MY_VAR: '$$response.body.token' } to store runtime values as environment variables for subsequent steps
Use setViewportSize() to set inner viewport dimensions, which calculates delta from window size rather than setting window size directly
Treat maxVariation percentage values as decimals (0.1 = 10%) but multiply by 100 when performing percentage-based comparisons
File overwrite modes support three values: 'false' (never overwrite), 'true' (always overwrite), 'aboveVariation' (overwrite only if content differs > maxVariation)

Files:

  • src/tests/httpRequest.js
  • src/tests/saveScreenshot.js
src/tests/httpRequest.js

📄 CodeRabbit inference engine (AGENTS.md)

Reference OpenAPI operations by operationId (e.g., step.httpRequest.openApi.operationId = 'getUserById') rather than path/method combinations

Files:

  • src/tests/httpRequest.js
🧠 Learnings (6)
📓 Common learnings
Learnt from: CR
Repo: doc-detective/core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T19:31:27.066Z
Learning: Applies to src/tests/**/*.js : Treat `maxVariation` percentage values as decimals (0.1 = 10%) but multiply by 100 when performing percentage-based comparisons
📚 Learning: 2025-11-26T19:31:27.066Z
Learnt from: CR
Repo: doc-detective/core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T19:31:27.066Z
Learning: Applies to src/tests/**/*.js : Treat `maxVariation` percentage values as decimals (0.1 = 10%) but multiply by 100 when performing percentage-based comparisons

Applied to files:

  • src/tests/httpRequest.js
  • src/tests/saveScreenshot.js
📚 Learning: 2025-11-26T19:31:27.066Z
Learnt from: CR
Repo: doc-detective/core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T19:31:27.066Z
Learning: Applies to src/tests/**/*.js : File overwrite modes support three values: 'false' (never overwrite), 'true' (always overwrite), 'aboveVariation' (overwrite only if content differs > maxVariation)

Applied to files:

  • src/tests/httpRequest.js
  • src/tests/saveScreenshot.js
📚 Learning: 2025-11-26T19:31:27.066Z
Learnt from: CR
Repo: doc-detective/core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T19:31:27.066Z
Learning: Applies to src/tests/httpRequest.js : Reference OpenAPI operations by `operationId` (e.g., `step.httpRequest.openApi.operationId = 'getUserById'`) rather than path/method combinations

Applied to files:

  • src/tests/httpRequest.js
📚 Learning: 2025-11-26T19:31:27.066Z
Learnt from: CR
Repo: doc-detective/core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T19:31:27.066Z
Learning: Applies to src/openapi.js : In OpenAPI integration, extract request/response examples from spec, validate payloads via AJV against schemas, and support mock responses by setting `step.httpRequest.openApi.mockResponse = true`

Applied to files:

  • src/tests/httpRequest.js
📚 Learning: 2025-11-26T19:31:27.066Z
Learnt from: CR
Repo: doc-detective/core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T19:31:27.066Z
Learning: Applies to src/tests/**/*.js : Use `setViewportSize()` to set inner viewport dimensions, which calculates delta from window size rather than setting window size directly

Applied to files:

  • src/tests/saveScreenshot.js
🧬 Code graph analysis (2)
src/tests/httpRequest.js (2)
src/tests/runShell.js (5)
  • require (1-1)
  • require (2-6)
  • result (15-25)
  • fractionalDiff (138-141)
  • existingFile (135-135)
src/utils.js (1)
  • fractionalDiff (279-279)
src/tests/saveScreenshot.js (2)
src/tests/runShell.js (2)
  • fractionalDiff (138-141)
  • result (15-25)
src/utils.js (1)
  • fractionalDiff (279-279)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
  • GitHub Check: test (macos-latest, 24)
  • GitHub Check: test (ubuntu-latest, 20)
  • GitHub Check: test (macos-latest, 20)
  • GitHub Check: test (macos-latest, 22)
  • GitHub Check: test (windows-latest, 24)
  • GitHub Check: test (windows-latest, 22)
  • GitHub Check: test (windows-latest, 20)
  • GitHub Check: test (ubuntu-latest, 24)
  • GitHub Check: test (ubuntu-latest, 22)
🔇 Additional comments (3)
src/tests/saveScreenshot.js (1)

289-366: LGTM! Fractional difference standardization implemented correctly.

The conversion from percentage-based to fractional-based difference calculation is well-executed:

  • Line 339 correctly calculates fractionalDiff as numDiffPixels / (width * height) (0-1 scale) instead of multiplying by 100 (0-100 scale)
  • The comparison at line 347 now directly compares fractional values without scaling
  • Log messages and user-facing descriptions consistently use fractional formatting with .toFixed(2)

This standardization simplifies the logic by keeping both fractionalDiff and maxVariation on the same 0-1 scale throughout.

Based on learnings, the previous approach required multiplying by 100 for percentage-based comparisons, but this PR eliminates that complexity.

src/tests/httpRequest.js (2)

7-7: LGTM! Import updated for renamed function.

The import correctly reflects the function rename from calculatePercentageDifference to calculateFractionalDifference.


456-480: LGTM! Fractional difference logic correctly implemented.

The changes properly convert from percentage-based to fractional-based comparison:

  • Function call updated to calculateFractionalDifference (line 457)
  • Direct comparison of fractional values at line 463
  • Messages correctly format fractional values with .toFixed(2)
  • Logic is consistent with the same changes in saveScreenshot.js

The default maxVariation of 0 (line 188) appropriately enforces exact matches for HTTP responses unless explicitly configured otherwise.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR standardizes the handling of variation thresholds across the codebase by refactoring from percentage-based (0-100) to fractional-based (0-1) calculations. The function calculatePercentageDifference() is renamed to calculateFractionalDifference() and now returns a decimal fraction instead of a percentage string. This change eliminates inconsistencies where some code paths were multiplying by 100 while others were not, making the comparison logic clearer and more uniform.

Key changes:

  • Renamed calculatePercentageDifference() to calculateFractionalDifference() with updated return type (number 0-1 instead of percentage string)
  • Updated all variation threshold comparisons across test utilities to use fractional values directly
  • Improved user-facing messages to display fractional differences with 2 decimal places

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/utils.js Renamed function to calculateFractionalDifference, added JSDoc, removed percentage conversion, added empty string edge case handling, returns raw fractional value
src/tests/saveScreenshot.js Updated variable names, comparison logic, and warning/success messages to use fractional values instead of percentages
src/tests/runShell.js Updated variable names, comparison logic, log messages, and error messages to use fractional values instead of percentages
src/tests/httpRequest.js Updated variable names, comparison logic, log messages, error messages to use fractional values; removed erroneous * 100 multiplication; includes minor formatting cleanup
AGENTS.md Updated documentation to clarify that maxVariation uses fractional values (0.1 = 10% tolerance) and comparisons use fractions directly

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1763a44 and 01defc1.

📒 Files selected for processing (4)
  • AGENTS.md (5 hunks)
  • src/tests/httpRequest.js (6 hunks)
  • src/tests/runShell.js (3 hunks)
  • src/tests/saveScreenshot.js (5 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
  • src/tests/runShell.js
  • src/tests/httpRequest.js
  • src/tests/saveScreenshot.js
🧰 Additional context used
🧠 Learnings (17)
📓 Common learnings
Learnt from: CR
Repo: doc-detective/core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T19:31:27.066Z
Learning: Applies to src/tests/**/*.js : Treat `maxVariation` percentage values as decimals (0.1 = 10%) but multiply by 100 when performing percentage-based comparisons
📚 Learning: 2025-11-26T19:31:27.066Z
Learnt from: CR
Repo: doc-detective/core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T19:31:27.066Z
Learning: Update Resolver workflow syncs dependencies: installs specified resolver version + matching `doc-detective-common` version, runs tests, bumps patch version, creates release with aggregated PR notes and resolver changelog

Applied to files:

  • AGENTS.md
📚 Learning: 2025-11-26T19:31:27.066Z
Learnt from: CR
Repo: doc-detective/core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T19:31:27.066Z
Learning: Applies to src/tests.js : Core test runner `src/tests.js` orchestrates Appium/WebDriver sessions and manages spec → test → context → step hierarchy (600+ LOC orchestrator)

Applied to files:

  • AGENTS.md
📚 Learning: 2025-11-26T19:31:27.066Z
Learnt from: CR
Repo: doc-detective/core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T19:31:27.066Z
Learning: Input resolution via `doc-detective-resolver` detects tests from docs/specs and resolves to executable format as first step in test execution flow

Applied to files:

  • AGENTS.md
📚 Learning: 2025-11-26T19:31:27.066Z
Learnt from: CR
Repo: doc-detective/core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T19:31:27.066Z
Learning: Tests follow nested hierarchy: spec (file) → test → context (browser/platform combo) → step (action), with contexts running serially and skipping if unsupported

Applied to files:

  • AGENTS.md
📚 Learning: 2025-11-26T19:31:27.066Z
Learnt from: CR
Repo: doc-detective/core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T19:31:27.066Z
Learning: Test & Publish workflow runs cross-platform tests (Ubuntu/Windows/macOS × Node 18/20/22/24) before publishing; triggers downstream `doc-detective` update via repository dispatch

Applied to files:

  • AGENTS.md
📚 Learning: 2025-11-26T19:31:27.066Z
Learnt from: CR
Repo: doc-detective/core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T19:31:27.066Z
Learning: Each step type has dedicated handler module in `src/tests/` (e.g., `httpRequest.js`, `runShell.js`) implementing specific action logic

Applied to files:

  • AGENTS.md
📚 Learning: 2025-11-26T19:31:27.066Z
Learnt from: CR
Repo: doc-detective/core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T19:31:27.066Z
Learning: Applies to src/index.js : Entry point `src/index.js` must expose `runTests()` function as the primary API for test execution

Applied to files:

  • AGENTS.md
📚 Learning: 2025-11-26T19:31:27.066Z
Learnt from: CR
Repo: doc-detective/core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T19:31:27.066Z
Learning: Run local test suite with `npm test` (mocha), use `node dev` for development/manual testing, run `npm run depcheck` to check unused dependencies

Applied to files:

  • AGENTS.md
📚 Learning: 2025-11-26T19:31:27.066Z
Learnt from: CR
Repo: doc-detective/core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T19:31:27.066Z
Learning: Auto Dev Release workflow skips on `[skip ci]` commits, release commits, or docs-only changes; increments dev version format `X.Y.Z-dev.N`

Applied to files:

  • AGENTS.md
📚 Learning: 2025-11-26T19:31:27.066Z
Learnt from: CR
Repo: doc-detective/core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T19:31:27.066Z
Learning: Applies to src/tests.js : Register new step types in `src/tests.js`: add action to `driverActions` array if requires browser, add case in `runStep()` switch statement

Applied to files:

  • AGENTS.md
📚 Learning: 2025-11-26T19:31:27.066Z
Learnt from: CR
Repo: doc-detective/core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T19:31:27.066Z
Learning: Applies to scripts/postinstall.js : Post-install script (`scripts/postinstall.js`) must auto-download Chrome/Firefox/ChromeDriver to `browser-snapshots/` directory matching current platform

Applied to files:

  • AGENTS.md
📚 Learning: 2025-11-26T19:31:27.066Z
Learnt from: CR
Repo: doc-detective/core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T19:31:27.066Z
Learning: Applies to src/tests/*.js : Create new step type handlers in `src/tests/[actionName].js` exporting async functions that validate schema, resolve to object, set defaults, and return `{ status: 'PASS'|'FAIL'|'WARNING', description: string, outputs: {} }`

Applied to files:

  • AGENTS.md
📚 Learning: 2025-11-26T19:31:27.066Z
Learnt from: CR
Repo: doc-detective/core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T19:31:27.066Z
Learning: Applies to src/tests.js : Configure Appium drivers (`chromium`, `gecko`, `safari` on macOS) with default 10-minute timeout via `newCommandTimeout: 600`

Applied to files:

  • AGENTS.md
📚 Learning: 2025-11-26T19:31:27.066Z
Learnt from: CR
Repo: doc-detective/core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T19:31:27.066Z
Learning: Applies to src/tests/**/*.js : Set step variables via `step.variables = { MY_VAR: '$$response.body.token' }` to store runtime values as environment variables for subsequent steps

Applied to files:

  • AGENTS.md
📚 Learning: 2025-11-26T19:31:27.066Z
Learnt from: CR
Repo: doc-detective/core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T19:31:27.066Z
Learning: Applies to src/tests/**/*.js : Always handle driver cleanup in try/finally blocks to ensure WebDriver sessions are properly terminated

Applied to files:

  • AGENTS.md
📚 Learning: 2025-11-26T19:31:27.066Z
Learnt from: CR
Repo: doc-detective/core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T19:31:27.066Z
Learning: Applies to src/config.js : Validate configuration objects via `doc-detective-common` schemas using `validate({ schemaKey: 'config_v3', object })`

Applied to files:

  • AGENTS.md
🪛 markdownlint-cli2 (0.18.1)
AGENTS.md

88-88: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
  • GitHub Check: test (macos-latest, 24)
  • GitHub Check: test (windows-latest, 24)
  • GitHub Check: test (windows-latest, 20)
  • GitHub Check: test (windows-latest, 22)
  • GitHub Check: test (macos-latest, 20)
  • GitHub Check: test (macos-latest, 22)
  • GitHub Check: test (ubuntu-latest, 24)
  • GitHub Check: test (ubuntu-latest, 22)
  • GitHub Check: test (ubuntu-latest, 20)
🔇 Additional comments (1)
AGENTS.md (1)

130-130: Documentation accurately reflects the fractional comparison logic.

The documentation on line 130 correctly states that maxVariation uses fractional values directly (0.1 = 10% tolerance) without percentage conversion. This is confirmed by the calculateFractionalDifference() function in src/utils.js, which explicitly documents: "Compare against maxVariation thresholds directly (e.g., 0.1 for 10% tolerance)." The PR successfully standardizes to fractional differences (0–1) throughout the codebase.

@hawkeyexl hawkeyexl merged commit 580f4d0 into main Dec 3, 2025
23 of 49 checks passed
@hawkeyexl hawkeyexl deleted the maxvariation-standardization branch December 3, 2025 00:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug report: maxVariation Is interpreted as percent instead of fraction

2 participants