Validate and triage the agent-feedback backlog - #3714
Open
DylanPiercey wants to merge 2 commits into
Open
Conversation
…d off out-of-repo items
|
This was referenced Jul 31, 2026
WalkthroughThe change updates the agent-feedback filing rules and adds a categorized won’t-fix catalog. It replaces the bug, cleanup, DX, and unclear findings with refreshed entries. It adds performance investigations for runtime, compiler, serialization, resumability, and bundle behavior. The entries include updated diagnostics, validation requirements, documentation corrections, test-harness changes, cleanup tasks, and re-verification details. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (1)
agent-feedback/perf.md (1)
83-88: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winDefine the cache-key contract for
extraArgs.If
extraArgsis recreated on every render, the proposed cache misses. If it is mutated in place, reusing the cached closure can capture stale arguments. Specify whetherextraArgsis immutable and identity-stable, or key the cache by stable argument values. Re-verify both stable and changing argument cases.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@agent-feedback/perf.md` around lines 83 - 88, The cache introduced around addTagsEvents/bindTagsEventHandler must define and enforce an extraArgs key contract: reuse the bound handler only when extraArgs identity and contents remain valid, while creating a new handler when arguments change or are mutated. Preserve stable function identity across renders for unchanged on-x("method") bindings, and verify both stable extraArgs and changing/mutated argument cases.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@agent-feedback/bugs.md`:
- Line 213: Fix the Markdown formatting in agent-feedback/bugs.md at the
affected inline code span by removing unintended leading or trailing spaces
inside the backticks. Preserve the code text and surrounding explanation.
- Line 357: Fix the MD038 violation in the documentation around
preservingWhitespaceUntil by removing any leading or trailing whitespace inside
the affected inline code span. Keep it inline unless the content requires
intentional spacing, in which case convert it to a fenced code block.
In `@agent-feedback/cleanup.md`:
- Line 57: Verify the accessor utility rename by searching both the old
filename/import form “get-accessor-char” and symbol form “getAccessorChar”
before and after renaming the module to reflect its current exports. Update
every stale import path and confirm no old file or references remain, while
preserving getAccessorPrefix and getAccessorProp usage.
In `@agent-feedback/dx.md`:
- Around line 65-70: Update the planned optional-helper tests to validate the
intended Sorted.isSuperset relation rather than preserving the known-broken
equal-list false result; adjust the assertion alongside the isSuperset
implementation fix and retain any legacy behavior check only as a temporary
regression marker. Add the pure helper coverage in optional.test.ts, and record
this actionable agent-feedback finding in agent-feedback/ before finishing.
- Around line 11-16: Restrict the truncated-operator diagnostic in parser.js
onText to known expression-valued attributes such as if, const, let, and
expression attributes, leaving ordinary markup like `<div class=x > 0>yes</div>`
valid. Add a regression covering valid text after a tag close and verify the
intended expression case still reports an error. Record this actionable finding
in the appropriate agent-feedback/ file before finishing.
- Around line 137-142: The SSR controllable assertion in writeControlledScope
hardcodes valueChange instead of naming the handler for each ControlledType.
Under MARKO_DEBUG, derive the handler attribute name from ControlledType and
pass it to assertHandlerIsFunction, preserving the shared InputValue behavior
used by _attr_textarea_value and the existing six SSR helpers.
- Around line 29-34: Update the event-handler guidance in cheatsheet.md to
document the supported two-parameter form, showing that the handler element is
accessed through the second parameter rather than event.currentTarget or
event.target. Do not change AttrEventHandler typings or attempt to customize the
TypeScript diagnostic; preserve the existing event-handler examples and add the
corresponding two-argument example near the onInput guidance.
In `@agent-feedback/perf.md`:
- Around line 107-112: Update the non-aliased `<const>` handling in `analyze`
and its `translate.exit` value-emission path so unread initializers are not
discarded solely because they have zero readers. Use the existing side-effect
analysis to eliminate both evaluation and storage only for proven
side-effect-free initializers; otherwise preserve initializer evaluation while
omitting the stored binding value.
- Around line 23-28: Only elide custom-tag child wiring and imports for
all-constant call sites when child analysis proves the child is client-inert
after SSR. Extend the child analysis exported alongside
childExtra.domExports?.setupEmpty to represent absence of client-observable
effects, event handlers, DOM updates, and resume work; preserve wiring for any
such behavior or top-level side effects, and verify the two-instance dom
compilation.
In `@agent-feedback/unclear.md`:
- Around line 35-40: Update the reproduction in agent-feedback/unclear.md to
include the badge span inside the header after the dynamic style, verifying
following-sibling selector scope and an empty diagnostic result. Retain a
separate parent or cross-subtree example that exercises the warning boundary,
and document both updated cases.
---
Nitpick comments:
In `@agent-feedback/perf.md`:
- Around line 83-88: The cache introduced around
addTagsEvents/bindTagsEventHandler must define and enforce an extraArgs key
contract: reuse the bound handler only when extraArgs identity and contents
remain valid, while creating a new handler when arguments change or are mutated.
Preserve stable function identity across renders for unchanged on-x("method")
bindings, and verify both stable extraArgs and changing/mutated argument cases.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 76b0a333-305a-45fb-b1ef-20641f4c87e5
📒 Files selected for processing (7)
agent-feedback/README.mdagent-feedback/bugs.mdagent-feedback/cleanup.mdagent-feedback/dx.mdagent-feedback/perf.mdagent-feedback/unclear.mdagent-feedback/wontfix.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Every one of the 213 entries was re-checked against current source: the cited symbol still exists, the claimed mechanism still holds, and it wasn't already fixed. Descriptions are rewritten to the house 2–6 sentence format, which halves the active backlog (47k → 24k words) without losing a repro or a path.
Five entries are deleted as verified stale or wrong. Forty-four move to a new
wontfix.md— verified findings whose cost/benefit didn't justify the work, each with a Why not now line so the same ground isn't re-surveyed. Six move out to the repos that actually own the fix (htmljs-parser, language-server, testing-library, website), with the marko-side remainder kept here where the work genuinely splits.No true duplicates existed. Cross-references between related entries are restored and made reciprocal — the shortening had dropped 17 of them.
Review pass. All 11 CodeRabbit comments were checked against the source and answered inline. Six led to entry edits — two padded code spans, a search recipe that used a symbol that never existed, a false-positive caveat on the
>heuristic (every Marko attribute value is an expression, so "restrict to expression attributes" excludes nothing), a handler-parameter correction, and a soundness precondition on the inert-child perf entry. Five were rejected with evidence; the rest of the pass also cleared six older malformed code spans the linter surfaced.