Skip to content

Add AI instructions#1430

Open
AlCalzone wants to merge 1 commit intohome-assistant-libs:mainfrom
AlCalzone:ai-instructions
Open

Add AI instructions#1430
AlCalzone wants to merge 1 commit intohome-assistant-libs:mainfrom
AlCalzone:ai-instructions

Conversation

@AlCalzone
Copy link
Copy Markdown
Contributor

These have been compiled from recent review comments.

Comment thread AGENTS.md
Look for prior art in `zwave_js_server/model/` before inventing. Examples reviewers have flagged:

- `from_dict` classmethod parses server payload: `def from_dict(cls, data: <Type>DataType) -> Self`. Returns `Self`, not the explicit class.
- Rename unused parameters to `_param` (e.g. `event` in event handlers).
Copy link
Copy Markdown
Contributor

@MartinHjelmare MartinHjelmare May 6, 2026

Choose a reason for hiding this comment

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

I don't understand this. We don't have any parameters called _param.

Copy link
Copy Markdown
Contributor Author

@AlCalzone AlCalzone May 6, 2026

Choose a reason for hiding this comment

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

The auto-generated instructions specifially mentioned _event but I wanted to keep it more generic. If it only ever comes up in event handlers, I can go back to being more specific.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't like this. We should keep the interface parameters always named the same. I suggest we remove this instruction and adjust the parameter names instead.

Comment thread AGENTS.md

Look for prior art in `zwave_js_server/model/` before inventing. Examples reviewers have flagged:

- `from_dict` classmethod parses server payload: `def from_dict(cls, data: <Type>DataType) -> Self`. Returns `Self`, not the explicit class.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is just best practice Python type annotation using modern Python.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Right, but you flagged it during your most recent review. Opus 4.7 didn't follow best practices, so I think it needs a nudge in the right direction.

Copy link
Copy Markdown
Contributor

@MartinHjelmare MartinHjelmare May 6, 2026

Choose a reason for hiding this comment

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

Yes. Would it work to tell it to follow the latest type annotation best practices (proposed in PEPs) of the minimum supported version of the library?

typing.Self was added in Python 3.11. We support Python 3.12+.

https://docs.python.org/3/library/typing.html#typing.Self

https://peps.python.org/pep-0673/

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@abmantis do you have any experience with this approach?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Its hard to say. Since its already "old" (3.11) it may work, but it is probably going to be flaky/inconsistent since its very broad.

As a side note: don't we have mypy/pyright/ruff/... rules we can use for these?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

don't we have mypy/pyright/ruff/... rules we can use for these?

That would probably be the more reliable option.

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.

3 participants