-
Notifications
You must be signed in to change notification settings - Fork 16
Refactor and simplify events module #100
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
- Simplify Event base class __init__ logic - Fix and improve type annotations (use lowercase dict, add Optional/bool types) - Remove redundant code and comments in ServerEvent subclasses - Extract omit() utility to separate utils.py module - Improve timestamp handling consistency across Event classes - Fix incorrect docstrings (Remove class) - Clean up varargs type hints (*elements: Element instead of List[Element]) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the final PR Bugbot will review for you during this billing cycle
Your free Bugbot reviews will reset on December 3
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| self.etype = etype | ||
| self.__dict__.update(kwargs) | ||
| def __init__(self, **kwargs): | ||
| super().__init__(**kwargs) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Missing default values for ClientEvent attributes
The refactoring removed default values for etype and value attributes in ClientEvent. The old code set self.etype = None when not provided and had a class attribute value = None. The new code only sets these via kwargs, causing AttributeError when accessed but not provided. This breaks backward compatibility since __eq__ (line 36), __repr__ (line 43), and UPLOAD handling (line 48-54) all access these attributes, expecting them to exist.
Additional Locations (1)
|
|
||
| def __init__(self, data, etype: Optional[str] = None, ts: Optional[float] = None, **kwargs): | ||
| super().__init__(ts=ts, **kwargs) | ||
| self.data = data |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: ServerEvent etype not serialized from class attribute
The refactoring removed self.__dict__.update(etype=self.etype, **kwargs) from ServerEvent.__init__. This line copied the class attribute etype into the instance's __dict__, ensuring it was included in serialization. Without it, when etype is only a class attribute (as in Noop, Set, Update, etc.), it won't be in self.__dict__ and won't be included in the serialized output from _serialize(). The client requires etype to identify event types, so this breaks event communication.
Additional Locations (1)
marvinluo1
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
Summary
Eventclass with a unified__init__that handles timestamp conversion centrallyClientEventandServerEventto usesuper().__init__()properly with cleaner inheritanceomitutility function fromhtml_components.pyto a newutils.pyfor better code organizationTest plan
Note
Centralizes event initialization/timestamp handling, streamlines event constructors and typing, and moves
omithelper to a newutils.pywith updated imports.Event.__init__with centralized timestamp handling;ClientEvent/ServerEventnow callsuper().__init__.Update/Add/Upsert/Removeconstructors and added consistent type hints; preserved_serializebehavior.NullEventdefaultetypeandts=None;Frameframe_rate: float;ServerRPCUUID handling clarified.html_components.py: Remove inlineomit; import fromvuer.utils.utils.pyintroducingomitand used byhtml_components.py.