Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .coderabbit.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
inheritance: false
reviews:
profile: assertive
review_details: true
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Strong suggestion: explicitly wire emberTemplateLint so the preview actually exercises the tool path.

With inheritance: false and no reviews.tools section, this config never explicitly enables the ember-template-lint integration. Empirically, the 4 CodeRabbit inline comments on this PR (bad.hbs:7/:13/:32/:52) contain zero 🪛 ember-template-lint tool blocks and zero 🧰 Tools blocks — they read as generic HTML/accessibility observations from the LLM reviewer, not tool-backed output. By comparison, PR #97 (Preview/Stylelint) surfaces two 🪛 Stylelint (17.9.0) blocks, proving tool-backed output is identifiable when it actually runs.

The second commit (1d636b6 "show tooling in review detils") added review_details: true, which is a sensible step but doesn't add the tool entry the preview needs. Adding review_details: true alone does not turn on a tool that the config has not explicitly enabled.

The canonical tool key is emberTemplateLint (camelCase) per the CodeRabbit schema.v2.json reviews.tools properties — ember-template-lint (kebab-case) is not a recognized key. Note that PR #96 (Preview/EmberTemplate)'s current head ships the kebab-case form, which is also why that branch has zero 🪛 ember-template-lint tool blocks despite explicit wiring; PR #96 is a more explicit failed attempt, while this PR has even less wiring than that.

Suggested config:

inheritance: false
reviews:
  profile: assertive
  review_details: true
  tools:
    emberTemplateLint:
      enabled: true

Severity: strong suggestion (not a blocker — this is a demo/preview branch that doesn't merge).

7 changes: 7 additions & 0 deletions .template-lintrc.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"rules": {
"no-bare-strings": "error",
"require-button-type": "error",
"require-valid-alt-text": "error"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Strong suggestion: add at least one Ember-specific rule + matching fixture so tool provenance is distinguishable.

The three enabled rules (no-bare-strings, require-button-type, require-valid-alt-text) overlap entirely with generic HTML/accessibility advice that any LLM reviewer would surface, so any review output on this branch can be fully explained by the LLM reading bad.hbs rather than by ember-template-lint actually running. Concrete evidence on this PR: CodeRabbit's bad.hbs:13 inline comment flags label-input association, which would map to require-input-label — but that rule is not in this config. That's an LLM observation layered on top of the configured rule set, not strict tool output.

To make tool provenance unambiguous, add at least one rule that an LLM is unlikely to flag from generic HTML knowledge alone, plus a matching fixture line in bad.hbs:

{
  "rules": {
    "no-bare-strings": "error",
    "require-button-type": "error",
    "require-valid-alt-text": "error",
    "no-implicit-this": "error",
    "deprecated-each-syntax": "error"
  }
}

Then add corresponding bait, e.g.:

  • <h1>Hello {{formatName}}!</h1> (triggers no-implicit-this — wants {{this.formatName}})
  • {{#each items}}{{this}}{{/each}} (triggers deprecated-each-syntax — wants {{#each items as |item|}})

Once one of those Ember-specific rule names appears in a CodeRabbit 🪛 ember-template-lint (X.Y.Z) block, the preview is demonstrably exercising the tool path rather than just the generic LLM reviewer.

Severity: strong suggestion (not a blocker — this is a demo/preview branch that doesn't merge).

}
}
53 changes: 53 additions & 0 deletions bad.hbs
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
<article>
<img src="product.jpg">
<h3>Premium Widget</h3>
<p>Only $29.99 — Order now and save 20%!</p>
<span>Free shipping on all orders</span>
<button>Add to Cart</button>
<button>Buy Now</button>
Comment on lines +6 to +7
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

find . -name "bad.hbs" -o -name "*.hbs" | head -20

Repository: coderabbitai/coderabbit-pr-review

Length of output: 87


🏁 Script executed:

cat -n bad.hbs

Repository: coderabbitai/coderabbit-pr-review

Length of output: 1646


Buttons inside <form> must declare explicit types to prevent accidental form submissions.

Lines 14-15 lack type attributes:

  • <button>Log In</button> should be type="submit"
  • <button>Reset</button> should be type="reset"

Buttons at lines 6-7, 32, 42, 48, and 51 are not inside form elements, so they don't pose a form submission risk.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@bad.hbs` around lines 6 - 7, The "Log In" and "Reset" button elements must
declare explicit types to avoid accidental form submissions—locate the <button>
element with text "Log In" and change it to include type="submit", and locate
the <button> element with text "Reset" and change it to include type="reset";
leave the other buttons (e.g., "Add to Cart", "Buy Now" and those not inside a
form) unchanged.

</article>
<form>
<label>Username</label>
<input type="text" placeholder="Enter username">
<label>Password</label>
<input type="password">
Comment on lines +10 to +13
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

find . -name "bad.hbs" -type f

Repository: coderabbitai/coderabbit-pr-review

Length of output: 87


🏁 Script executed:

cat -n bad.hbs

Repository: coderabbitai/coderabbit-pr-review

Length of output: 1646


🏁 Script executed:

rg -t handlebars '{{t\s' --max-count=5

Repository: coderabbitai/coderabbit-pr-review

Length of output: 158


🏁 Script executed:

rg -t handlebars '\{\{t ' --max-count=5

Repository: coderabbitai/coderabbit-pr-review

Length of output: 112


🏁 Script executed:

rg '\{\{t ' -g '*.hbs' --max-count=10

Repository: coderabbitai/coderabbit-pr-review

Length of output: 59


🏁 Script executed:

fd -e hbs -type f

Repository: coderabbitai/coderabbit-pr-review

Length of output: 248


🏁 Script executed:

fd '\\.hbs$'

Repository: coderabbitai/coderabbit-pr-review

Length of output: 59


🏁 Script executed:

git ls-files | head -20

Repository: coderabbitai/coderabbit-pr-review

Length of output: 133


🏁 Script executed:

cat .template-lintrc.json && echo "---" && cat README.md

Repository: coderabbitai/coderabbit-pr-review

Length of output: 229


Associate labels with inputs for accessible form navigation.

Lines 10 and 12 labels are not bound to corresponding inputs. Add for/id pairs to establish the association.

Suggested fix
-  <label>Username</label>
-  <input type="text" placeholder="Enter username">
-  <label>Password</label>
-  <input type="password">
+  <label for="username">Username</label>
+  <input id="username" type="text" placeholder="Enter username">
+  <label for="password">Password</label>
+  <input id="password" type="password">
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<label>Username</label>
<input type="text" placeholder="Enter username">
<label>Password</label>
<input type="password">
<label for="username">Username</label>
<input id="username" type="text" placeholder="Enter username">
<label for="password">Password</label>
<input id="password" type="password">
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@bad.hbs` around lines 10 - 13, The labels "Username" and "Password" are not
associated with their inputs; update the <label> and corresponding <input>
elements so each label has a for="..." that matches an id="..." on its input
(e.g., label for="username" with input id="username", and label for="password"
with input id="password"), ensuring the input types and placeholders remain
unchanged and adding unique ids if needed to avoid collisions.

<button>Log In</button>
<button>Reset</button>
<p>Forgot your password?</p>
</form>
<section>
<h2>Photo Gallery</h2>
<div>
<img src="sunset.jpg">
<p>A beautiful sunset over the mountains</p>
</div>
<div>
<img src="beach.jpg">
<p>Sandy beach with crystal clear water</p>
</div>
<div>
<img src="forest.jpg">
<p>Dense forest trail in autumn</p>
</div>
<button>Load More Photos</button>Collapse comment
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Remove accidental text artifact on Line 32.

Collapse comment appears to be unintended UI content appended after the button.

🧹 Proposed fix
-  <button>Load More Photos</button>Collapse comment
+  <button type="button">{{t "gallery.load_more"}}</button>
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<button>Load More Photos</button>Collapse comment
<button type="button">{{t "gallery.load_more"}}</button>
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@bad.hbs` at line 32, Remove the accidental "Collapse comment" text that was
appended after the <button> element containing "Load More Photos" in the bad.hbs
template; locate the <button>Load More Photos</button> instance and delete the
trailing "Collapse comment" artifact so only the intended button markup remains.

</section>
<nav>
<h1>My Application</h1>
<ul>
<li>Home</li>
<li>About Us</li>
<li>Services</li>
<li>Contact</li>
</ul>
<button>Menu</button>
<img src="avatar.png">
</nav>
<div>
<h1>Welcome to our site</h1>
<p>Click the button below</p>
<button>Submit</button>
<img src="hero.jpg">
<span>Contact us</span>
<button>Cancel</button>
<img src="icon.png">
Comment on lines +3 to +52
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

file="$(fd '^bad\.hbs$' | head -n1)"
echo "Inspecting: $file"

echo -e "\nButtons without explicit type:"
rg -nP '<button(?![^>]*\btype=)[^>]*>' "$file" || true

echo -e "\nImages without alt:"
rg -nP '<img(?![^>]*\balt=)[^>]*>' "$file" || true

echo -e "\nPotential bare strings between tags:"
rg -nP '>\s*[A-Za-z][^<{]*<' "$file" || true

echo -e "\nLabels without for=:"
rg -nP '<label(?![^>]*\bfor=)[^>]*>' "$file" || true

Repository: coderabbitai/coderabbit-pr-review

Length of output: 1593


Localize hardcoded UI copy to comply with the configured no-bare-strings rule.

All text content across lines 3–52 must be wrapped in a localization helper (e.g., {{t "..."}}) or the template will fail linting. Additionally:

  • Buttons (lines 6, 7, 14, 15, 32, 42, 48, 51): Add type="button" attribute
  • Images (lines 2, 21, 25, 29, 43, 49, 52): Add alt attribute for accessibility
  • Labels (lines 10, 12): Add for= attribute to associate with input fields
  • Line 32: Remove the orphaned text node "Collapse comment" or wrap it properly
Proposed localization pattern
-  <h3>Premium Widget</h3>
-  <p>Only $29.99 — Order now and save 20%!</p>
-  <button>Add to Cart</button>
+  <h3>{{t "product.premium_widget.title"}}</h3>
+  <p>{{t "product.premium_widget.promo_text"}}</p>
+  <button type="button">{{t "actions.add_to_cart"}}</button>
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@bad.hbs` around lines 3 - 52, The template contains hardcoded UI strings and
missing attributes that break linting and accessibility: wrap every visible
string in the localization helper (e.g., change "Premium Widget", "Only $29.99 —
Order now and save 20%!", every <p>, <span>, <h1>/<h2>/<h3>, button labels like
"Add to Cart", "Buy Now", "Log In", "Reset", "Load More Photos", "Menu",
"Submit", "Cancel", and image captions with {{t "..."}}), add type="button" to
all bare <button> elements (buttons with text "Add to Cart", "Buy Now", "Log
In", "Reset", "Load More Photos", "Menu", "Submit", "Cancel"), add meaningful
alt attributes to all <img> tags (e.g., avatar.png, hero.jpg, icon.png,
sunset.jpg, beach.jpg, forest.jpg), add for="..." on <label> elements and
corresponding id="..." on their <input> elements (labels "Username" and
"Password" -> link to their inputs), and remove or properly wrap the orphaned
text node "Collapse comment" (delete it or include it inside a localized
element). Ensure these changes reference the existing template elements
(headings h1/h2/h3, <p>, <span>, <button>, <img>, <label>, <input>) so linters
and accessibility checks pass.

</div>