Improve private notes voice flows#255
Conversation
Signed-off-by: Uzair Ullah <uzairullahmail@gmail.com>
Added a guideline to use plain spoken English without special formatting. Signed-off-by: Uzair Ullah <uzairullahmail@gmail.com>
✅ Community PR Path Check — PassedAll changed files are inside the |
🔀 Branch Merge CheckPR direction: ✅ Passed — |
✅ Ability Validation Passed |
🔍 Lint Results✅
|
Ju-usc
changed the title
There was a problem hiding this comment.
Pull request overview
This PR introduces an improved community/private-notes voice-first notes ability for OpenHome, using a constrained “tool loop” design where the LLM selects an action and Python executes it (including confirmations for destructive actions).
Changes:
- Added a new
PrivateNotesCapabilityimplementing write/read/search/delete note flows with Python-owned confirmations and safer storage normalization. - Implemented JSON file persistence (
private_notes.json) with strict validation and safe overwrite behavior (delete then write). - Added ability documentation (
README.md) and a more detailed design/spec (SPEC.md).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| community/private-notes/main.py | Implements the private notes tool-loop, notebook persistence/validation, and confirmation-gated mutations. |
| community/private-notes/README.md | Documents user-facing behavior, storage model, and trigger phrases. |
| community/private-notes/SPEC.md | Captures architecture, data model, tool contracts, and safety rules for the ability. |
| community/private-notes/init.py | Adds the package init file required by ability validation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if tool_name == "ask_followup": | ||
| question = tool_args.get("question", "") | ||
| if self._looks_like_destructive_confirmation(question): | ||
| history.append({ | ||
| "role": "user", | ||
| "content": ( | ||
| "Do not ask confirmation with ask_followup. " | ||
| "Python handles delete and overwrite confirmation. " | ||
| "Call delete_notes or write_note with the matching note ids now." | ||
| ), | ||
| }) | ||
| continue | ||
|
|
There was a problem hiding this comment.
_looks_like_destructive_confirmation() flags any ask_followup question containing words like "update"/"replace" as a destructive confirmation. This will incorrectly suppress legitimate clarification questions (e.g., “What would you like to update in that note?”), pushing the model into mutation tools prematurely and breaking update flows. Tighten detection to confirmation-style phrasing (e.g., yes/no questions like “Are you sure…”, “Do you want me to delete/overwrite…”) rather than keyword-matching action verbs.
| # Capture time once so the context prefix stays identical across turns (LLM caching). | ||
| now = datetime.now(ZoneInfo(self.capability_worker.get_timezone())) | ||
|
|
There was a problem hiding this comment.
now is captured once at ability start and then reused for all tool executions, which means note created_at/updated_at timestamps reflect the initial trigger time even if the user spends time in follow-ups / confirmations. Consider keeping a stable context_now for prompt caching, but generating a fresh datetime.now(...) (or ISO timestamp) at the moment a mutation executes.
| notes = sorted( | ||
| notebook["notes"], key=lambda n: n.get("updated_at", ""), reverse=True | ||
| ) | ||
| note_index = { | ||
| "note_count": len(notes), | ||
| "notes": [ | ||
| {"id": n.get("id"), "title": n.get("title"), "updated_at": n.get("updated_at")} | ||
| for n in notes | ||
| ], | ||
| } | ||
| return ( | ||
| f"Current local time: {now.isoformat()}\n" | ||
| f"User request: {request_text}\n" | ||
| f"Note index:\n{json.dumps(note_index, ensure_ascii=True)}" | ||
| ) |
There was a problem hiding this comment.
The initial LLM context includes the full note index for every stored note (id/title/updated_at). As the notebook grows, this will increase prompt size/latency and can eventually exceed model context limits, causing failures or degraded behavior. Consider capping the index (e.g., most recent N notes) and using search_notes/ask_followup to reach older notes when needed.
| # Capture time once so the context prefix stays identical across turns (LLM caching). | ||
| now = datetime.now(ZoneInfo(self.capability_worker.get_timezone())) | ||
|
|
There was a problem hiding this comment.
ZoneInfo(self.capability_worker.get_timezone()) can raise if get_timezone() returns None or an invalid tz name (common in other abilities, which fall back to a default). Consider tz_name = self.capability_worker.get_timezone() or "UTC" and catching ZoneInfoNotFoundError to fall back safely, so the ability doesn't fail on missing/unknown timezone data.
|
Closing this conflicted branch in favor of the clean replacement PR: #256 |
What does this Ability do?
Improves the existing
community/private-notesvoice-first note ability. It adds topic search, safer note mutation handling, Python-owned destructive confirmations, and clearer voice UX expectations.Suggested Trigger Words
Type
External APIs
Testing
Additional validation:
python3 -m py_compile community/private-notes/main.pypython3 validate_ability.py community/private-notesgit diff --check -- community/private-notes/main.py community/private-notes/README.md community/private-notes/SPEC.mdChecklist
community/private-notes/main.pyfollows SDK pattern (extendsMatchingCapability, hasregister_capability+call)README.mdincluded with description, suggested triggers, and setupresume_normal_flow()called on every exit pathprint()— usingeditor_logging_handlerredis,user_config)asyncio.sleep()orasyncio.create_task()— usingsession_tasksAnything else?
Runtime notes:
openhome-cli assign --jsonreported success whilestatusstill showed emptypersonality_ids, so the live chat transcript was treated as the stronger attachment proof.Delete endpoint not yet implemented.