Skip to content

fix(ui): count and filter every kept form event in the Forms timeline - #290

Merged
erkamyaman merged 2 commits into
mainfrom
fix/h-panel
Oct 11, 2026
Merged

erkamyaman merged 2 commits into
mainfrom
fix/h-panel

Conversation

@erkamyaman

@erkamyaman erkamyaman commented Oct 11, 2026 •

Copy link
Copy Markdown
Member

What and why

selectedEvents in app/src/pages/forms-inspector.ts filtered events to the selected form and then cut them with a fixed .slice(-200). The server already caps events at limits.formTimeline (default 200, up to 2000), and the timeline draws at most 100 matches. So the fixed slice added nothing; it only threw away kept events before filtering and counting.

Anyone who raised limits.formTimeline got a wrong Timeline:

  • the count stopped at 200 ("Showing the latest 100 of 200 events" with 300 kept)
  • the path, type and origin filters couldn't find older events and showed "No events match these filters"
  • no limit note appeared, because the server hadn't dropped anything

The fixed slice is removed. The server's limit decides how many events are kept, and the timeline still draws at most 100.

How it was verified

New "FormsInspector timeline" test in app/src/__tests__/forms-panels.test.ts with 300 kept events and formTimeline: 500. It checks the "of 300" count, and that a path filter finds 50 older events. On main it shows "of 200". With the fix it passes. All panel and package tests pass.

  • pnpm commit:check (commit messages follow the guidelines)
  • pnpm format:check
  • pnpm typecheck (includes the ngc template checks)
  • pnpm test:panel and pnpm test:devtools (all passing)
  • pnpm skills:check (when .claude/ changed): not changed
  • No docs change needed: the docs already describe this behavior (no-docs)
  • pnpm extension:build and extension/ui committed
  • Checked in the browser with axe (when the UI changed): logic change only, no markup or style change

Summary by CodeRabbit

  • Bug Fixes
    • The Forms Inspector now displays all events matching the selected form, instead of limiting the results to the most recent 200.
    • Filtering the timeline by form name shows all matching events, even when the timeline initially displays only the latest events.

The panel cut the selected form's events to the newest 200 before the
timeline filtered and counted them. The server already keeps events up to
limits.formTimeline (up to 2000) and the timeline draws at most 100, so the
extra cut only hid kept events: the count stopped at 200 and the path, type
and origin filters could not find older events. The fixed slice is gone.
@erkamyaman erkamyaman added the no-docs This pull request needs no docs change (the reason is in the description) label Oct 11, 2026
@erkamyaman erkamyaman added the no-docs This pull request needs no docs change (the reason is in the description) label Oct 11, 2026
@github-actions github-actions Bot added area: panel The devtools panel app (app/) area: extension The Chrome extension labels Oct 11, 2026
@coderabbitai

coderabbitai Bot commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: e53e5e6b-ce2f-4daf-83bf-c87a3bbbe231

📥 Commits

Reviewing files that changed from the base of the PR and between 3e0807b and 0926b3f.


⛔ Files ignored due to path filters (1)
  • extension/ui/assets/index-B6EjRaMv.js is excluded by !**/assets/index-[0-9a-z_-][0-9a-z_-][0-9a-z_-][0-9a-z_-][0-9a-z_-][0-9a-z_-][0-9a-z_-][0-9a-z_-].js

📒 Files selected for processing (4)
  • app/src/__tests__/forms-panels.test.ts
  • app/src/pages/forms-inspector.ts
  • extension/ui/assets/browser-agent-rpc-BXhoSh1z-MC1ssMuG.js
  • extension/ui/index.html

 __________________________________________________________
< Keep your friends close, but your code reviewers closer. >
 ----------------------------------------------------------
  \
   \   \
        \ /\
        ( )
      .( o ).

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 5b72f99d-4872-48a4-bc12-cfab8ed60e76


📥 Commits

Reviewing files that changed from the base of the PR and between 58273c6 and 3e0807b.



⛔ Files ignored due to path filters (1)
  • extension/ui/assets/index-oV-AD6Da.js is excluded by !**/assets/index-[0-9a-z_-][0-9a-z_-][0-9a-z_-][0-9a-z_-][0-9a-z_-][0-9a-z_-][0-9a-z_-][0-9a-z_-].js


📒 Files selected for processing (4)
  • app/src/__tests__/forms-panels.test.ts
  • app/src/pages/forms-inspector.ts
  • extension/ui/assets/browser-agent-rpc-BXhoSh1z-Dsu8xe_M.js
  • extension/ui/index.html


Limit details: You’ve used all 10 included reviews currently available.




📝 Walkthrough
📝 Walkthrough

Walkthrough

The forms inspector now returns all events matching the selected form ID. A timeline test checks the displayed events before and after filtering. The UI entry points reference a different JavaScript asset.

Changes

Forms timeline

Layer / File(s) Summary
Form event selection and timeline test
app/src/pages/forms-inspector.ts, app/src/__tests__/forms-panels.test.ts
selectedEvents returns all events matching the selected form ID. The test checks that the timeline initially displays the latest 100 of 300 events, then displays 50 matching events after filtering while retaining the total of 300.
UI asset references
extension/ui/assets/browser-agent-rpc-BXhoSh1z-Dsu8xe_M.js, extension/ui/index.html
The browser-agent RPC bundle import and the HTML module script now reference index-oV-AD6Da.js.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix



Merge Risk: ⚪ Minimal · up to 3e080

Retained form events beyond 200 can now appear in timeline counts and filters. No concrete issue remains that blocks merging.

Pre-merge checks | Passed 4 | Failed 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 3 files. (1 skipped: 1 … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: counting and filtering all retained form events in the Forms timeline.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.


Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 3 files. (1 skipped: 1 unsupported.)




  • Fix all pre-merge checks with AI
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR



🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Oct 11, 2026 •

Copy link
Copy Markdown

🚀 Deploying Preview to Cloudflare 🚀

Preview Deployments by commit

Status Deployment URL Commit Updated (UTC) See this deployment's details
  • Build: Failed ❌

View logs ↗
0926b3f 2026-10-11T07:55:25.141Z View logs ↗
  • Build: Failed ❌

View logs ↗
3e0807b 2026-10-11T03:22:13.306Z View logs ↗

# Conflicts:
#	extension/ui/assets/browser-agent-rpc-BXhoSh1z-CQUbrXfP.js
#	extension/ui/assets/browser-agent-rpc-BXhoSh1z-CufAYnYw.js
#	extension/ui/assets/browser-agent-rpc-BXhoSh1z-Dsu8xe_M.js
#	extension/ui/assets/index-CgvJVwtz.js
#	extension/ui/assets/index-EXPZcZbM.js
#	extension/ui/assets/index-oV-AD6Da.js
#	extension/ui/index.html
@erkamyaman
erkamyaman merged commit 71450fa into main Oct 11, 2026
6 of 8 checks passed
@erkamyaman
erkamyaman deleted the fix/h-panel branch October 11, 2026 07:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: extension The Chrome extension area: panel The devtools panel app (app/) no-docs This pull request needs no docs change (the reason is in the description)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant