Integrate selfie clean#2590
Open
WenjinFu wants to merge 7 commits into
Open
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR expands OM1’s face-memory workflow by adding two corrective actions (correct_identity, forget_last) and refactoring the existing selfie connector to better map face API outcomes into a unified SelfieStatus + TTS feedback channel.
Changes:
- Added new face-memory correction actions and connectors:
correct_identity(rename/merge) andforget_last(undo last enrollment). - Refactored
selfieconnector to improve response-code handling, add display-name cleanup, and adjust gating/timeout behavior. - Added a new “selfietest” greeting-mode config with an updated prompt that documents the enrollment/correction decision tree.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| src/actions/selfie/interface.py | Documents selfie parameters and adds force; (needs alignment of timeout defaults with connector). |
| src/actions/selfie/connector/selfie.py | Major connector refactor: new response dispatch table, pre-check behavior, state breadcrumbs, and TTS/status mapping. |
| src/actions/forget_last/interface.py | Defines the interface schema and usage notes for undoing the last enrollment. |
| src/actions/forget_last/connector/forget_last.py | Implements /gallery/forget_last connector with status + TTS mapping. |
| src/actions/correct_identity/interface.py | Defines the interface schema and usage notes for renaming/merging a recent enrollment. |
| src/actions/correct_identity/connector/correct_identity.py | Implements /gallery/move_samples connector with status + TTS mapping. |
| config/greeting_conversation-selfietest.json5 | Adds a demo/test config and prompt updates describing the new face-memory action flow. |
Comment on lines
40
to
+42
| action: str | ||
| timeout_sec: int = 5 | ||
| force: bool = False |
Comment on lines
+25
to
+29
| timeout_sec : int | ||
| Max seconds to wait for at least one face to appear before giving up. | ||
| Default 5. Bump higher (e.g. 10) for hesitant users. Note: the /selfie | ||
| API itself takes ~1.5s once started (multi-frame collection window), | ||
| so total worst-case latency is roughly `timeout_sec + 1.5s`. |
Comment on lines
+184
to
+188
| """Surface the result to the LLM as a SelfieStatus input line.""" | ||
| try: | ||
| self.io_provider.add_input("SelfieStatus", line, time.time()) | ||
| except Exception as e: | ||
| logging.warning("SelfieStatus write failed: %s", e) |
Comment on lines
+263
to
+267
| if err == "face_belongs_to": | ||
| matched = str(resp.get("name", "someone")) | ||
| sim = float(resp.get("sim", 0.0)) | ||
| matched_display = self._display_name(matched) | ||
| self.last_match_name = matched |
Comment on lines
344
to
348
| if not name: | ||
| logging.error("Selfie requires a non-empty `id` (e.g., 'wendy').") | ||
| self.io_provider.add_input("SelfieStatus", "failed reason=bad_id", time.time()) | ||
| logging.error("[Selfie] empty id; expected something like 'wendy'") | ||
| self._write_status("result=bad_id detail=empty") | ||
| # No TTS — LLM should have produced a valid id | ||
| return |
Comment on lines
66
to
69
| timeout_sec: int = Field( | ||
| default=15, | ||
| description="Default timeout in seconds for operations.", | ||
| default=8, | ||
| description="Max seconds to wait for ≥1 face before giving up.", | ||
| ) |
Comment on lines
+85
to
+88
| try: | ||
| self.io_provider.add_input("SelfieStatus", line, time.time()) | ||
| except Exception as e: | ||
| logging.warning("SelfieStatus write failed: %s", e) |
Comment on lines
+80
to
+83
| try: | ||
| self.io_provider.add_input("SelfieStatus", line, time.time()) | ||
| except Exception as e: | ||
| logging.warning("SelfieStatus write failed: %s", e) |
Comment on lines
+111
to
+115
| async def connect(self, output_interface: CorrectIdentityInput) -> None: | ||
| """Execute a single identity rename/merge.""" | ||
| from_id = (output_interface.from_id or "").strip().lower() | ||
| to_id = (output_interface.to_id or "").strip().lower() | ||
|
|
Comment on lines
+96
to
+100
| async def connect(self, output_interface: ForgetLastInput) -> None: | ||
| """Execute a single undo of the most recent enrollment.""" | ||
| body: Dict = {} | ||
| id_check = getattr(output_interface, "id", None) | ||
| if id_check: |
02c4d0a to
fb11f53
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces two new connectors for face memory correction actions (
correct_identityandforget_last) and improves the documentation and robustness of the existingselfieconnector. The main changes include new stateless connectors that interact with the face API for renaming or forgetting recent enrollments, new interface definitions for these actions, and enhanced docstrings and parameter descriptions for clarity and maintainability.New connectors for face memory correction:
CorrectIdentityConnectorincorrect_identity.py: Implements stateless logic to rename or merge a recently-enrolled identity by calling/gallery/move_sampleson the face API, with robust error handling and status/voice feedback.ForgetLastConnectorinforget_last.py: Implements stateless logic to undo the most recent enrollment by calling/gallery/forget_laston the face API, with comprehensive status and voice feedback.New interface definitions:
CorrectIdentityInputandCorrectIdentitydataclasses incorrect_identity/interface.pyto define the input/output schema and document usage for label correction actions.ForgetLastInputandForgetLastdataclasses inforget_last/interface.pyto define the input/output schema and document usage for undoing enrollments.Selfie connector improvements:
selfie.py, including more precise timeout settings and better separation of responsibilities (enrollment vs. correction). [1] [2] [3] [4] [5] [6] [7]selfieconnector to improve TTS output clarity.These changes make the face memory actions more robust, modular, and easier to maintain, while providing clearer feedback to both the LLM and the user.