Repository navigation
Fix fresh-chat send acknowledgement false negative #54
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1576,13 +1576,17 @@ async def _read_assistant_count_baseline(self) -> int: | |
| # Unreachable (the loop either returns or raises). | ||
| raise SendReadinessError("send_baseline: exhausted retries unexpectedly") | ||
|
|
||
| async def _verify_send_acknowledged(self) -> bool | None: | ||
| async def _verify_send_acknowledged(self, *, fresh_chat: bool = False) -> bool | None: | ||
| """P0 send acknowledgment (ChatGPT review, conv 6a52f0f3). | ||
|
|
||
| After click_send dispatches synthetic mouse events, verify the message | ||
| was actually accepted by React — not just that the JS event loop ran. | ||
|
|
||
| Composite condition: user-message count increased AND composer cleared. | ||
| On a fresh chat only, a transition to a valid ``/c/{uuid}`` URL with | ||
| an empty composer is also positive acknowledgement. ChatGPT creates | ||
| that conversation URL only after accepting the first submission, and | ||
| its user-message DOM node can hydrate after the SPA navigation. | ||
| Uses the pre-send user count baseline (self._pre_send_user_count) to | ||
| detect the delta, not just "userCount > 0" (which is always true on | ||
| existing conversations). | ||
|
|
@@ -1615,7 +1619,7 @@ async def _verify_send_acknowledged(self) -> bool | None: | |
| f" || document.querySelector('{COMPOSER_FALLBACK_SELECTOR}');" | ||
| " var composerPresent = !!composer;" | ||
| " var composerEmpty = composer ? !(composer.innerText || composer.value || '').trim() : false;" | ||
| " return JSON.stringify({userCount: userMsgs, composerPresent: composerPresent, composerEmpty: composerEmpty});" | ||
| " return JSON.stringify({userCount: userMsgs, composerPresent: composerPresent, composerEmpty: composerEmpty, href: location.href});" | ||
| "})()" | ||
| ) | ||
| if not result or not result.strip().startswith("{"): | ||
|
|
@@ -1634,6 +1638,20 @@ async def _verify_send_acknowledged(self) -> bool | None: | |
| current_count = state.get("userCount", 0) | ||
| if current_count > pre_send_count and state.get("composerEmpty"): | ||
| return True | ||
| if fresh_chat and state.get("composerEmpty"): | ||
| # Fresh-chat /c/{uuid} navigation can precede user DOM hydration. | ||
| import re as _re | ||
| import uuid as _uuid | ||
| match = _re.search( | ||
| r"/c/([0-9a-fA-F-]{36})(?:[/?#]|$)", | ||
| str(state.get("href") or ""), | ||
| ) | ||
| if match: | ||
| try: | ||
| _uuid.UUID(match.group(1)) | ||
| return True | ||
| except ValueError: | ||
| pass | ||
| except Exception: | ||
| pass | ||
| await asyncio.sleep(0.5) | ||
|
|
@@ -1791,7 +1809,9 @@ async def send_and_stream( | |
| # could prevent sends in edge cases we haven't seen. | ||
| if not captured_uuid: | ||
| try: | ||
| acknowledged = await self._verify_send_acknowledged() | ||
| acknowledged = await self._verify_send_acknowledged( | ||
| fresh_chat=(fallback_anchor.mode == "fresh_chat"), | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [MEDIUM] No regression test for the caller-side fresh_chat wiring The crux of the fix in production is the call site |
||
| ) | ||
| if acknowledged is False: # explicitly False, not None | ||
| raise SendReadinessError( | ||
| "Send not acknowledged — click dispatched but no user " | ||
|
|
||
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.
[LOW] URL validation is looser than a canonical UUID check
The regex
[0-9a-fA-F-]{36}accepts any 36 characters of hex digits and hyphens, anduuid.UUID()strips hyphens before validating, so it accepts non-canonical hyphen placement too. For example,/c/0123456789abcdef0123456789abcdef-abc(36 chars, 32 hex digits) passes both checks despite not being a canonical UUID form. Practical impact is minimal since real ChatGPT conversation URLs are canonical 8-4-4-4-12, but if the intent is 'valid /c/{uuid}', a canonical regex such as[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}would make the gate exact and let you drop the redundant uuid.UUID round-trip.