feat: skill quality overhaul — 95% faster E2EE, 28 evals, enhanced harness#17
feat: skill quality overhaul — 95% faster E2EE, 28 evals, enhanced harness#17
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request delivers substantial improvements to the Matrix communication skill, focusing on both user experience and developer maintainability. The core change dramatically boosts E2EE message performance by optimizing room resolution, making interactions significantly faster. Concurrently, the documentation has been overhauled to be more comprehensive and compliant, while the testing harness and evaluation suite have been expanded to ensure higher quality and reliability across all operations. These changes collectively enhance the skill's efficiency, robustness, and ease of use. Highlights
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request significantly enhances the Matrix skill by updating its documentation, improving the harness verification script, and refining the Python scripts for Matrix operations. Key changes include adding new keywords to plugin.json, extensively updating AGENTS.md and SKILL.md with more detailed commands, rules, error handling, and common mistakes, and greatly expanding the evaluation coverage in evals.json. The verify-harness.sh script is now more robust, checking for skill-specific consistency, versions, and script health. Performance notes in e2ee-guide.md and script usage notes were updated to reflect faster E2EE sync times. Crucially, the Python scripts for E2EE operations (matrix-edit-e2ee.py, matrix-read-e2ee.py, matrix-send-e2ee.py) were refactored to use a new find_room_in_nio_client function, resolving room names more efficiently by leveraging already synced client data, thereby reducing HTTP calls. Additionally, line buffering was enabled for real-time output in most Python scripts. There are two feedback items: one regarding an outdated E2EE performance note in AGENTS.md that needs to be consistent with other documentation, and another suggesting a simplification and efficiency improvement for the room finding logic in skills/matrix-communication/scripts/_lib/rooms.py.
- Rewrite description to pure triggering conditions (no capability list) - Reduce word count from 818 to 479 (under 500 CI limit) - Add all send/read/edit flags as explicit Quick Reference examples - Add Common Mistakes section, expand Error Handling table (10 entries) - Add set +H && to first send example for bash ! safety - Restore source link in References
Root cause: find_room_by_name() made 193 HTTP requests (2 per room x 96 joined rooms) before every operation. The sync itself only took ~0.8s. Fix: E2EE scripts now sync first (timeout=0, full_state=True), then resolve room names from client.rooms (in-memory, zero HTTP calls). - Add find_room_in_nio_client() to _lib/rooms.py for post-sync lookup - Refactor send/read/edit to resolve rooms after sync - Set sync timeout=0 (no long-poll) for immediate return - Update e2ee-guide.md performance notes (~2-3s, not ~5-10s)
- Add # /// script blocks with dependencies to 8 non-E2EE scripts - Add sys.stdout/stderr.reconfigure(line_buffering=True) after imports - Place reconfigure calls after all imports to avoid E402 lint errors - Ensures real-time output in non-interactive/piped contexts
AGENTS.md: - Full-path commands for all 15 scripts (no shorthand) - 11 development rules (E2EE first, bash !, key backup, etc.) - Updated performance notes (~2-5s, not ~5-10s) Evals: expanded from 3 to 28 covering all operations, error recovery, room search, key requests, device verification, and performance. plugin.json: added e2ee, encryption, messaging, notification, nio.
verify-harness.sh: - L1: Add SKILL.md existence check - L2: Frontmatter validation, plugin.json consistency, version sync, reference docs presence - L3: Eval coverage (min 5), eval script references, script syntax - Fix PR template check to handle missing API access in CI harness-verify.yml: delegate to script instead of inline checks.
d7abacf to
6128148
Compare
Summary
client.roomsinstead of 193 HTTP calls viafind_room_by_name()Performance
Root cause:
find_room_by_name()made 2 HTTP requests × 96 joined rooms = 193 requests before every operation. Fix: sync first (timeout=0), resolve fromclient.rooms(in-memory).Test plan
uvx ruff check .anduvx ruff format --check .pass clean