Skip to content

feat(ai): recent console errors capture + prompt sandwich#360

Open
dobrinyonkov wants to merge 16 commits into
masterfrom
ai-improvements-v1
Open

feat(ai): recent console errors capture + prompt sandwich#360
dobrinyonkov wants to merge 16 commits into
masterfrom
ai-improvements-v1

Conversation

@dobrinyonkov

Copy link
Copy Markdown
Contributor

Stacked on #357.

What's in here

Eleven commits since `2145a54`, split into three concerns:

Recent Console Errors — injected-script layer

  • `72bf386` `consoleErrorBuffer` — bounded FIFO (capacity 3) of deduplicated console errors and warnings. Pure state machine, no browser deps.
  • `d9a7e29` `consoleErrorCapture` — browser-side glue that monkey-patches `console.error`/`console.warn` and hooks `window.onerror`/`unhandledrejection`, feeding events into the buffer. Install-once guarded.
  • `a7919c1` — wires the injected buffer through to the panel: injected pushes a snapshot on every event, panel caches it, AssistantController reads via `getConsoleErrors` seam per send.

Recent Console Errors — panel/AI layer

  • `5315eae` — PromptBuilder renders the `Recent Console Errors` sub-section (newest-first, `(×N)` for repeats, indented `at ` line, per-section cap).
  • `d9edad7` — AssistantController grows `getConsoleErrors`/`clearConsoleErrors` seams. `clearConversation` and `setUrl` reset both signals in lock-step.

Prompt-builder refactor (foundation for above)

  • `16e273b` — `buildUserPrompt` rewrite: sandwich structure (`User asked:` / middle / `Now answer:`) with shape-driven curation instead of a JSON dump. Bindings render as `prop ← "path" = value (model: …, type: …, formatter: yes)`; aggregations degrade from listing IDs to a type histogram at >3 children.
  • `6777984` — `const`/`let` sweep in the new helpers.

Docs & cleanup

  • `895b74a` `chore`: gitignore installed agent skills.
  • `0c2d409`, `d92920e`, `193ffab` — comment passes: remove obvious/verbose ones, tighten survivors, then expand JSDoc with rationale (why the caps, why the sandwich, why the install-once guard).

Tests

All 571 existing tests pass. New tests live alongside the new modules.

@dobrinyonkov dobrinyonkov force-pushed the refactor/ai-assistant-architecture-v1 branch from 2145a54 to 285be09 Compare July 2, 2026 09:18
Base automatically changed from refactor/ai-assistant-architecture-v1 to master July 3, 2026 11:59
Rewrite PromptBuilder.buildSystemPrompt around a small-model-friendly
shape: a one-sentence Role, a numbered Rules block (English-only reply,
grounding to the Current UI5 Control Context, prescribed uncertainty
phrase, runtime-data preference), a short Style block, and a copy-me
bad/good example instantiating the uncertainty phrase.

Enrich Current Application Context with three additional fields:
UI locale (configurationComputed.data.language), sap-ui-debug when
present in URL parameters, and Application entry point from
common.data.Application. Each line is omitted when its data is
unavailable, and the section is dropped entirely when appInfo has no
recognized fields.

Assertions cover: zone order, rule order and content, uncertainty
phrase template, the bad/good example, per-field presence/omission,
and four golden-file snapshots (no info, framework only,
framework+theme+libraries, all six fields).

No production behavior change beyond the emitted system prompt string.
…pe-driven curation

Replace the raw-JSON dumps of properties, bindings, and aggregations with
curated, one-line renderings, and wrap the user's question in a sandwich so
the small local model keeps focus on it after the context block.

New user-prompt shape when Inspection Context is attached:

  User asked: <msg>

  Current UI5 Control Context:
  - Type: ...
  - ID: ...
  Properties:
  - key: value
  Bindings:
  - prop <- "path" = value (model: ..., type: ..., formatter: yes)
  Aggregations:
  - name: N children (Type x N, Other x M)

  Now answer: <msg>

Per-section rules:
- Properties render as key: value lines; capped at 800 chars.
- Bindings render one line each with the resolved value inline when the
  snapshot carries it; null and undefined print literally; values truncate
  at ~100 chars; composite bindings (parts: []) collapse to a degenerate
  <composite> line; section capped at 800 chars.
- Aggregations render as: empty (empty), N children -- id1, id2 (<=3), or
  N children (Type x N, ...) (>3); section capped at 400 chars.
- Circular binding graphs still bail out to the pre-existing 'cannot
  serialize' placeholder.

No-context / no-control calls return the raw user message unchanged.

Existing PromptBuilder.spec.js invariant tests are updated to the new
shape; new golden-file tests cover identity-only, properties-only,
bindings, and mixed aggregations. Two AssistantController tests are
updated to assert 'Now answer:' instead of 'User Question:'.

Refs .scratch/ai-prompt-context-quality/issues/02-rewrite-user-prompt.md
The helper functions and module-scope constants added in 16e273b used
'var' — inconsistent with the rest of the file (and the codebase's
ES2020+ baseline). Switch to const/let. No behavior change.
Pure state machine over an error-event stream backing the AI Assistant's
Recent Console Errors signal. Bounded FIFO of capacity 3 with dedup by
(normalized message, top-shown stack frame) — duplicates increment the
count on the existing entry and do not re-promote.

Stack-frame selection skips up to 3 framework frames matching
'sap-ui-core.js' or 'resources/sap/', then falls back to whatever frame
we landed on. No browser dependencies — testable without a running page.

Part of .scratch/ai-prompt-context-quality/issues/03.
Thin adapter around consoleErrorBuffer. Monkey-patches console.error /
console.warn (originals continue to run) and subscribes to window.onerror
and unhandledrejection, funneling every event into the buffer.

An optional onRecord callback lets the injected main.js push a fresh
snapshot to the panel on every recorded event. install() is idempotent
across a re-injected content script.

Part of .scratch/ai-prompt-context-quality/issues/03.
buildUserPrompt gains a third argument for the recent-console-errors
snapshot. Non-empty renders a 'Recent Console Errors:' block inside the
sandwich, immediately after Current UI5 Control Context (or in its place
when no control is attached). Empty or missing snapshot omits the section
entirely. Backwards-compatible: existing 2-argument callers keep working.

Each entry renders the message with a '(×N)' annotation when count > 1
and — when a frame is present — an indented 'at <frame>' line beneath.
The section is capped at ~400 characters with the standard [truncated]
marker on overflow. Errors render newest-first (reverse of the buffer's
natural FIFO arrival order).

Part of .scratch/ai-prompt-context-quality/issues/03.
…ontroller

AssistantController accepts two new optional injected callbacks
mirroring the existing getAppInfo pattern:

- getConsoleErrors — called on every sendUserMessage; the snapshot is
  forwarded to PromptBuilder.buildUserPrompt as the third argument.
- clearConsoleErrors — called from clearConversation and setUrl(differentUrl)
  so buffered errors reset in lock-step with Conversation Memory and
  Inspection Context.

Both seams tolerate missing options, undefined return values, and
throws from broken panel wiring — the send flow and clear paths never
break because of a downstream integration.

Part of .scratch/ai-prompt-context-quality/issues/03.
…tant

End-to-end wiring for the recent-console-errors signal:

- The injected script installs consoleErrorCapture on load and pushes a
  fresh snapshot to the panel via the existing devtools port protocol
  ('on-console-errors-updated') on every recorded event.
- Panel main.js caches the snapshot in frameData[frameId].consoleErrors
  and wires getConsoleErrors / clearConsoleErrors to the AI Assistant.
- clearConsoleErrors sends 'do-clear-console-errors' to the injected
  script so the panel's cache and the page-side buffer stay in lock-step
  when the developer invokes Clear Conversation.
- AIChat forwards the two new callbacks into AssistantController.

Closes .scratch/ai-prompt-context-quality/issues/03.
…work

Delete comments that restate the next line, narrate the PRD, or repeat
what a constant name already says. Keep comments that explain non-obvious
'why' — races, external-quirk workarounds, historical constraints.

Touches only files from the previous 9 commits:
- PromptBuilder.js:            -117 lines
- consoleErrorBuffer.js:        -46 lines
- consoleErrorCapture.js:       -25 lines
- AssistantController.js:       -14 lines
- injected/main.js:              -4 lines
- devtools/panel/ui5/main.js:    -5 lines

No behavior change. 571/571 tests pass, lint clean.
Rewrite remaining comments using plain, direct language: no jargon
(sandwich, curated, funnel, lock-step, glue, adversarial), no
editorializing about what the model sees, no filler doc-block prose.
Shorten sentences and drop redundant qualifiers.

Same 6 files as the previous docs commit.
Enrich JSDoc and inline comments for the console-errors and PromptBuilder
work with more rationale (why the caps, why the sandwich, why the install-
once guard). Behavior unchanged.
This reverts commit 193ffab, restoring the deliberately-lean comment
state that d92920e had established across the six AI-work files.

Docs history on ai-improvements-v1:
  0c2d409 docs(ai): remove obvious/verbose comments  (initial cull)
  d92920e docs(ai): tighten surviving comments        (concise pass)
  193ffab docs(ai): expand doc comments               (re-expansion)
  this    revert of 193ffab                           (restore d92920e state)

The 193ffab re-expansion added multi-paragraph JSDoc rationale,
rendered example prompts in docstrings, and forward references to
follow-up issues. The d92920e state carried one-sentence summaries on
public functions and bare @private/@param/@returns on helpers, which
matches the density used elsewhere in this codebase.

Docs-only. No code logic touched. See .scratch/ai-prune-comments/PRD.md
for context.
Review-and-prune pass over the six files touched by the reverted 193ffab.
Heuristic: if a JSDoc block has more sentences than its function has
statements, cut prose. Four files needed no changes; two had cuts:

- AssistantController#on: drop "in-process event bus" restatement — the
  class docblock already declares no Chrome deps.
- AssistantController#sendUserMessage: shrink 3-sentence race-guard
  essay to the one-line invariant; the "do not overwrite session-failed"
  clause is enforced mechanically at the rejection handler below.
- AssistantController#updateInspectionContext: collapse numbered
  lifecycle list to prose. Same info, one sentence.
- PromptBuilder#_renderAggregationLine: drop rendered example prompt
  from docstring — tests cover output shape.
- PromptBuilder#_renderBindingLine: same.

No code logic changed. npm test: 582/582 pass.
…one method

Collapse the two AssistantController reseed-tracking methods into a single
_trackReseed(rawSeed, { announceReady = false } = {}). The pair only
differed in whether 'ready' was re-emitted on the Assistant Capability
State stream after a successful reseed.

Call sites:
- initialize, downloadModel: _trackReseed(seed) (unchanged)
- clearConversation, setUrl reseed: _trackReseed(seed, { announceReady: true })

The _pendingReseed write, the session-failed translation on rejection,
and the 'resurface ready when capability is currently ready' logic all
stay in one place now. Pure internal refactor; no public API, event, or
test assertion changed. All 582 tests pass.
When Chrome tears down the extension's background service worker during
idle (~30-60s of port inactivity), the Prompt API session dies with it.
Prior to this fix the next Send tripped PromptClient's 'No active
session' guard and surfaced as a streaming-failed banner; a second Send
recovered.

AssistantController now checks PromptClient.hasActiveSession() at the
top of sendUserMessage. If false and no reseed is already in flight,
it kicks off a reseed via _trackReseed(_seedSession()) before awaiting
the pending-reseed gate. The recovery reseed carries the current
Conversation Memory so follow-ups still reference earlier turns, and
the sticky Inspection Context (ADR-0002) applies to the recovered
prompt as usual. UI is silent — the first Send after idle just takes
slightly longer to start streaming.

Prefactor: _pendingReseed is now null when no reseed is in flight and
a Promise while one is (previously initialized to Promise.resolve() as
a sentinel that was never reset). All existing reseed origins funnel
through _trackReseed, which clears the field back to null on both
fulfillment and rejection. sendUserMessage reads it defensively via
(this._pendingReseed || Promise.resolve()).then(...).

PromptClient gains a hasActiveSession() getter over the private
_hasActiveSession flag. The existing onDisconnect handler that
synthesises a mid-stream error for in-flight streams stays untouched.

Tests: six new cases in tests/modules/ai/AssistantController.spec.js
exercise session-dead-on-Send, memory carried through recovery,
coalescing with an in-flight clearConversation reseed, recovery reseed
failure surfacing as session-failed (not streaming-failed), unchanged
alive-path behaviour, and Inspection Context surviving recovery. The
fake PromptClient gains a controllable hasActiveSession() (default
true).

Refs .scratch/ai-session-dies-when-idle/PRD.md
Refs .scratch/ai-session-dies-when-idle/issues/01-recover-from-idle-killed-session.md
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.

1 participant