Struggle detection v3 + proactive intervention (client) - #335
Draft
Predixx wants to merge 326 commits into
Draft
Conversation
…name the dismiss-pause The proactive on/off switch was a bare "On" pill with no visible purpose, and the auto-pause showed only "Auto-paused" without saying it was caused by dismissing hints. Add a visible "Proactive help" label plus a tooltip, and surface the pause as "Paused after dismissing recent hints". Spec section 12.2 / 5.2 updated to match.
…(re)detection Reopening VS Code on an already-cloned exercise only ran passive workspace detection (which activated Iris chat) but never started the struggle session, so the engine had no active exercise and stayed silent. Wire the seam-gated coordinator's start/end into the detection callbacks, symmetric with the active webview open flow: start on detect, end on no-match so a stale exercise cannot linger. endExerciseSession added to the IStruggleCoordinator seam + no-op coordinator.
…ventions An active struggle event carries an optional anchor (file/line/inlineHint), but onServerActive dropped it, so the client showed only the chat bubble plus toast and never the inline in-editor cue. Thread the anchor through the subscription, seam wiring, and orchestrator; render the inline breadcrumb (clearing any standing lamp, since inline and lamp are exclusive surfaces) when the anchor is live, per spec section 6.1. Capped actives still degrade to the lamp.
…bled When classifyIrisCourseAvailability lands on 'disabled', log which check failed: the profile/module feature not being active (with the actual activeProfiles plus activeModuleFeatures), or iris-settings.enabled being false. Diagnoses a case where the chat reported disabled while the course was actually enabled and proactive worked.
…on chat bubbles - Collapse a run of consecutive proactive Iris messages into one card with a "Show N earlier suggestions" toggle (display-only; detection/backoff untouched). - Move thumbs + Dismiss into a hover-revealed floating bar at the card's bottom edge (reserves no space, keyboard-reachable via :focus-within, no container chrome). - Subtle chevron/link collapse indicator instead of the dashed pill. - Spec 6.2/6.5/12.1 updated to match.
Both user and assistant bubbles were capped at 70% of the panel width, wasting horizontal space in the chat sidebar. Cap on min(92%, 70ch) instead: fill a narrow sidebar but keep line length readable when it is dragged wide.
- Drop the unused nextSensorSeq re-export from the sensing barrel (the function is imported directly from ./sequence, which is where it is used). - Un-export DEFAULT_EGRESS_CAPS and STRUGGLE_EVENT_TOPIC; both are only used inside their own modules.
Slice 1 (logging): extract the per-tick formatter into a pure, testable telemetry/formatTick module and enrich the [Struggle] line with the severity decomposition, every gate flag, and the warmup/cooldown countdowns derivable from the tick. Slice 2a (snapshot): add a separate StruggleDebugSnapshot wire type (not on TickRecord), ThrottledAlertSink.getThrottleState forwarded by BackoffGate, an engine grace getter, and StruggleCoordinator.getDebugSnapshot; logging Phase B appends the throttle / grace / fN2 timers to the same per-tick line. Slice 2b (dashboard): feed the snapshot through the per-tick struggleDetectionInit payload (developer mode only) and render a TimersPanel with an offset-corrected 1 s clock for smooth countdowns between the 10 s ticks. The panel shows a no-active-session empty state, and the provider refreshes it on session start/end (ticks stop when a session ends). The cooldown anchor derives from the firing tick's own alert because the engine emits onDidTick before onDidAlert. The clean Open VSX build stays leak-free (snapshot type lives in @shared, the no-op coordinator returns an inert snapshot).
While the engine is still warming up and the bar would otherwise read "armed", show "$(pulse) Struggle: warm-up M:SS" instead, counting down from tick time (warmupS - tick.t) so it never hits 0:00 before the first post-warm-up tick. firing/gated stay untouched (FM/E4 alerts break through warm-up and must remain visible); every tooltip notes the warm-up time remaining while it lasts. The display strings move into a pure formatAlertBar() in struggleAlertBarState (unit-tested, vscode-free); the status bar reads the warm-up length from the coordinator's debug snapshot caps, so the always-bundled bar never imports struggle/config and the clean build stays leak-free.
Add a Fullscreen button to the developer struggle view that opens it as a webview editor panel (which VS Code can natively move to a separate window). Per the chosen scope the embedded copy shows the dashboard/status/timers only, no live chart, so it needs no live-feed wiring: the panel is fed the SAME per-tick snapshot the sidebar uses, refreshed on every tick AND on session start/end so it never freezes on a stale session. A new `embedded` flag hides the back-link, live chart, and pop-out button in the panel; the loading state drops the back-link entirely (from the standalone panel it would mutate the sidebar's global state). Wiring: React posts toggleStruggleFullscreen -> navigationCommands -> facade delegates to a provider-supplied opener -> FullscreenPanelManager.openStruggleFullscreen (kept struggle-agnostic: it takes buildInit + subscribeRefresh closures). The coordinator access stays in the provider behind the @telemetry seam, so the always-bundled panel manager never imports the engine and the clean build stays leak-free. Also fix the status-bar warm-up readout to key off the engine's own inWarmup flag (t <= warmupS) instead of remaining > 0, so it persists through the final warm-up tick instead of disappearing one tick early.
… page developer-only Rework the developer struggle-detection view so the engine's decision is legible, and gate the whole page behind developer mode. - Add a decision-flow pipeline (Severity, Candidate, Gates, Outcome) at the top, fed by the init snapshot via a shared decision-trace mapper (toLiveDecisionTrace, reused by the live feed). The edit path is shown as four stages with the blocking stage highlighted; a discrete test-stagnation fire is shown as a separate verdict, not a faked all-pass. - Rewrite the timers panel in plain language (drop the B4/E6/fN2/S/V codes), add a "Last delivered" row from the throttle state, and a "waiting for first tick" re-arm state. Extract the per-second countdown math into useEngineCountdowns. - Remove the redundant Status card and the live-section CurrentTickPanel (plus its dead CSS); the pipeline covers the verdict, gates and boundary. - Gate the whole struggle page to developer mode: route guards on showStruggleDetection and openStruggleFullscreen, a dashboard entry gate via a required dashboardInit.hideDeveloperTools flag, and an in-view backstop. - Fix getSnapshot to return an inactive zero-state when no session is active, so the urgency meter no longer shows stale post-session data.
… entry label - Make the urgency card compact and move it above the decision-flow pipeline (one-line score + status, slim bar; the long explanation moves to a hover tooltip). - In the pipeline's gate list, distinguish "blocking" (the gate the engine actually recorded as the reason this tick) from "engaged" (its condition holds but the flow stopped at an earlier stage) and "clear", with a short caption. This removes the apparent contradiction where warm-up showed as engaged while the recorded reason was "no boundary". - Label the dashboard "Struggle Detection" entry with a small "Dev" badge, since the page is developer-only.
…pipeline The pipeline derived the Severity stage's "over/below threshold" label from the stop position. The engine checks the candidate (boundary) BEFORE the threshold (see alertStateMachine: 'no-candidate' is recorded first, 'below-threshold' second), so when the recorded reason was "no boundary" a below-theta urgency (e.g. 0.16, 0.59) was wrongly labelled "over threshold" and shown green. Each stage now reflects its OWN factual condition (Severity: urgency vs theta; Candidate: boundary present), while the engine's recorded reason marks the decisive blocker (red). A stage that is factually not-ok but not the recorded reason is shown neutral with its true label, so the Severity box no longer claims "over threshold" when urgency is below theta.
…it payload
The decision-flow pipeline rendered below-threshold twice (as the Severity
stage and as a row in the gate list), so the Gates stage box stayed neutral
while the gate-list row showed a red "blocking" state for the same condition.
The urgency threshold is now shown only as the Severity stage; the gate list
carries the five real delivery gates (B2, B4, D1, cooldown, re-arm). Updated
the list copy ("Delivery gates this tick") and the stale gate-order comment to
the engine's actual order.
Also trim isStruggling/v/s/primaryBoundary/lastAlertT from the
struggleDetectionInit message and StruggleData: nothing renders them since the
Status card was removed. The Urgency card keeps urgency; the engine-side
StruggleSnapshot is unchanged.
…me wording Bug A: the "Delivery gates this tick" list rendered amber "engaged" from the raw tick-time gate flags. On a FIRED tick the flow stops nowhere, yet warm-up/grace flags can still be true (FM/E4 break through warm-up, FM/FM+ survive the grace filter), so those rows showed "engaged" while the Gates stage box was green "all clear" and the verdict was "Alert fired". Guard engaged with reason !== 'fired' so every gate row reads "clear" on a fire. Bug B: the Outcome sub said "nudge sent", but a fired edit decision is upstream of delivery (coordinator gate, backoff, throttle caps) and may be dropped, which also contradicted the delivery counters in the same panel. Reword to the decision-level "alert raised"; "Alert fired" and the verdict stay. Robustness: derive GATE_REASONS from GATES so the stage box and the rows cannot drift, and cap the in-view live-tick buffer at 600 (mirroring the feed's cap) so a very long session does not grow the array and the chart markers without bound.
…ncy card state The test-stagnation entry claimed tests were "stuck at the same number" and fired when the count "has not increased" for N builds. The tracker actually fires when N consecutive builds fail to beat the best passing-test count seen so far, folding flat, regressed AND failed builds into "no progress". Reword text + tooltip to match (keeps the "Tests are stuck" prefix). Also fix two boundary-wording inaccuracies: theta "must rise above" to "must reach or exceed" (engine fires at urgency >= theta), and d1-warmup "a failed build" to "a build that failed without improving" (warm-up admits only FM and E4, not the improved-but-still-failing FM+). The Urgency card was the only panel without a no-session state: with no active exercise session the engine reports urgency 0, so the card showed a calm green "0.00 / Below alert threshold" while the pipeline and timers showed their empty states. Guard the card on debug.sessionActive and show an explicit empty state.
…fix chart width The decision-flow stage boxes use fixed dark backgrounds but read their text from VS Code foreground tokens, which turn dark in a light theme, so the stage values and labels rendered dark-on-dark and were unreadable (the boxes are intentionally always-dark filled chips, so their text is now fixed light). The gate rows sit on the theme-adaptive card surface, so their engaged/blocking colors and the idle dot move from dark-tuned hex to VS Code semantic tokens (errorForeground / editorWarning-foreground / widget-border) with hex fallbacks. The live chart was permanently stuck at its 600px fallback width: the ResizeObserver was set up in a mount-only effect, but the measured .chartFrame only mounts once ticks arrive, so the observer never attached and the chart was clipped to the narrower sidebar (newest ticks cut off). Use a callback ref that attaches the observer exactly when the frame mounts and disconnects on unmount. Also soften the chart grid (mid-gray at low opacity, subtle on both themes) and draw a dot for the single-tick case where a line has no segment to render.
The previous fix kept the stage chips dark and only lightened their text, which left dark boxes sitting on a light editor like a dark-mode island. Tint the chips with translucent semantic colors instead (neutral gray, green pass, red block) so they adapt: pale tints over a light card, subtle dark tints over a dark card. Text returns to VS Code tokens (foreground / errorForeground / charts-green) so it stays legible on either theme.
…inflight, reset latches, signal weighting) - BackoffGate drops non-edit / course-off / student-opt-out alerts above the throttle (shouldSuppress) so suppressed alerts no longer burn the per-session delivery budget. - A 'failed' POST result releases the in-flight slot, so a transient error no longer wedges proactive for 30s. - Per-session latches (404 / course-off) and the active cap clear only on resetSession() (new exercise), not reset() (settings toggle), so a config-off-then-on toggle cannot lift a latch or refill the cap. - buildStruggleSignal core contribution uses the v3 /2 mean (was the stale v2 /3), fixing the dominantComponents ranking sent to Pyris.
…spec Design spec for the slot-based continuity layer that fixes the random downgrade / lack-of-continuity in proactive struggle interventions: the slot model (PARKED vs DELIVERED), pull/push surfaces, one-case escalation, stale-ask resolution with interpreted replies, reload-safe folding, minimal DB footprint, the request/response DTOs, and the literature grounding.
…tale watchdog, termination bounds) Resolve the open design holes surfaced during review of the slot model: - Async/generation guard: bump slot_generation only on semantic transitions (not IN-SESSION / ask-visibility); add single-flight per intent for decide/confirmClose; buttons disable after first click. - Stale handling: explicit client-side stale watchdog (arm/reset/fire), staleWindowCount + STALE_WINDOW_MAX hard force-free ceiling, staleAsk cap 2; covers ask=false noops so termination is mechanically bounded. - Delivery vs persistence: DELIVERED = at show (protection immediate, never reverts to PARKED); persistence best-effort/retried. - confirmClose frees the slot immediately; ~5s timer is UI-only folding. - PARKED lifecycle made authoritative (click/replace/progress/stale). - Replace LLM reply classification with deterministic quick-reply buttons; free-text only resets the ABANDON timer (bounded ceiling). - DTOs to snake_case wire form; episode.hints[] membership defined; outcome row canonical/idempotent; null-outcome episodes reported as attrition, excluded from rate denominators.
… spec §12/§17 Add the slot-model implementation plan (Phases A/B/C across Pyris, Artemis, and the extension): the wire/backend contract, the pure-logic slot core (SlotManager, async/generation guard, stale watchdog, progress-close latch), and the surfaces/orchestrator/webview wiring. Driven to build-ready through repeated adversarial review. Spec edits made to keep the design consistent with the plan: - §12: outcome existence and read are episode-wide (the write target stays the earliest-sentAt row), so a terminal outcome is stable under out-of-order persistence of proactive rows. - §12: add a second nullable column proactive_client_message_id as the reveal idempotency key; the DB footprint is now two nullable columns plus two enum values. - §17: ratify the extension-to-Artemis confirmReason discriminator (progress / stale_solved / parked_progress) for the confirmClose mode.
… stripMarkdown tests (#345) The paren-in-URL test only checked not-throw and toContain, which passed trivially because a paren inside the URL makes the link pattern not match at all, leaving the markup raw. Assert the real output instead, correct the doc-comment overclaim about the fenced-code pass, and add a linearity test for large fenced-code opener runs.
Brings in the reconnect reconciliation for Iris streaming (#355 via #362), the one commit dev was ahead by. Conflict resolutions: - chatSessionService: dev extracted the message mapping into formatIrisMessages, which did not carry the proactive fields. Kept the refactor and moved origin/proactiveOutcome/proactiveEpisodeId into the helper, so both the live load and the new reconnect fetch keep them. Without this every proactive hint would come back from history as a plain assistant message and lose the episode id the reveal flow keys on. - chatMessageService: took dev's return values (sentMessageId, generation) but not its struggleContext parameter, which this branch removed and which SendMessageInput no longer carries. - extensionMessages, useChatStore, IrisChatView, useChatStore.test: both sides added entries at the same spot, unioned. - chatWebviewProviderReconnect.test: dropped the telemetryManager argument, that constructor parameter was removed on this branch in 8fb446a. Verification: check-types clean, eslint clean, vitest 1823/1823 passing (156 files, +21 from the merged dev tests). test:unit could not run in this environment: the vscode-test harness aborts before any test code with "listen EINVAL" because the user-data socket path exceeds the 103 character unix socket limit. It fails identically without this merge.
They were roughly 10k of the PR's added lines (about a quarter of the diff) and are working documents, not deliverables. The files stay on disk and are listed in .git/info/exclude so they remain available locally without being tracked. The one spec dev already tracks (2026-06-24-ws-statusbar-button-design.md) is untouched, removing it here would show up as a deletion in the PR.
Completes the docs removal. Unlike the other 23 this one exists on dev, so the PR now carries it as a deletion rather than simply not adding it. The file stays on disk and is covered by the same .git/info/exclude entry as the rest.
… list (#366) (#367) The collapsed proactive-episode row sat far below the conversation it belonged to, and the list read airy throughout. Both had the same root cause: vertical space was reserved by the children rather than owned by their container, and a large part of it existed only to host chrome that is invisible at rest. - The fold line, the "N earlier hints" summary and the timeline card each carried `margin: 8px 16px`. Margins do not collapse inside a flex container, so that stacked on the list's own gap and on the preceding item's padding: ~34px above a 12px row, ~46px after an open episode card. The containers now own the vertical rhythm through `gap`; the children keep only the horizontal 16px inset, which is load-bearing (it aligns them with the bubble text). - Every message reserved space twice for chrome the student cannot see: a feedback row inside the bubble (`opacity: 0`, but always in flow) and a timestamp positioned absolutely at `bottom: -18px`, which forced the list gap to 18px so it had somewhere to live. Both now share one always-mounted footer row per message, so the space is reserved once and the list gap can drop to 8px. Hover changes colour only, never layout. The row recedes via `--vscode-descriptionForeground` rather than opacity: opacity composites the text into whatever is behind it and cannot guarantee a contrast ratio, and a user row has no focusable child, so an opacity-based rest state would never lift for keyboard users. For the same reason the row sits outside the bubble, which `.proactiveDismissed` dims with `opacity: 0.6`. - Feedback buttons follow the Artemis web client, whose rate buttons are permanently visible: borderless, `padding: 4px 6px`, selection carried by colour and the filled icon instead of a border. - A proactive card floats its action bar 14px below its own bottom edge. The tighter rhythm no longer leaves room for that by accident, so cards that actually render a bar reserve the clearance explicitly, between the bubble and the footer row where the collision is. Body text drops to `line-height: 1.5` with 10px paragraphs, matching the client.
…led offer covering its error (#368) (#370) Two geometry problems left over from #367, both hover chrome that occupies layout space or overhangs it instead of being reserved once. **The episode timeline moved the whole list on hover.** `.foot` animated `max-height` from 0 to 28px plus a `margin-top`, which is real flow, so hovering any row grew it and pushed every following row and every following message down by ~34px. It is now taken out of flow into the spacer `.body` already reserves, so revealing it costs no height at all and only opacity animates. The grace window survives unchanged in intent: the transition delay still sits on the resting state, so the fade-out lingers ~0.4s and the pointer can still reach Dismiss. The last row has no spacer to borrow (`.rowLast .body` has no bottom padding), so an out-of-flow foot would have covered its own text. It stays in flow and is instead always present, receding by colour like every other footer in the chat after #367. `.footPersistent` is pinned to `position: static` defensively: today every actionable foot is on the latest row, but a bar with real buttons must never float over the row above it if that changes. The spacer is 16px, not the previous 14px, and the foot pins the same 16px line box. At 14px the 11px timestamp overflowed by 1.4px, because the inherited line-height is 1.4, and grew upward into the row's text. `.time` also stops wrapping: out of flow it has no height budget beyond that one line. **A failed proactive offer covered its own error row.** `showOfferButtons` did not exclude `isFailed`, unlike `showDismiss` directly above it, so a send that never reached the server still rendered its floating action bar, 14px below the bubble, over an error footer starting 4px below it. There is nothing to answer in that state anyway.
* feat(iris-chat): refresh preserves a still-valid explicit session selection (#364) * feat(iris-chat): provider reveal-navigation; reveal owns focus (#364) * feat(struggle): reveal persists then navigates to the hint's exercise on confirmed persist (#364) * style(iris-chat): replace em dashes in #364 comments (#364) * refactor(struggle): drop reveal dead code orphaned by #364 (#364) generateLocalId and postRevealBubble lost their last callers when the parked-reveal path switched to a deterministic localId and stopped posting an optimistic bubble. Remove both from the telemetry engine-deps contract, StruggleInterventionDeps, the adapter, the extension.ts wiring, and all fakes. * feat(struggle): notify the student when a reveal permanently fails to persist (#364) A parked-hint reveal that gives up (permanent 4xx or retry cap reached) previously left the slot DELIVERED with no bubble, no navigation, and no feedback, so the student saw nothing and could not re-reveal. Surface a short warning on both give-up branches, guarded by the same consent-epoch check as the rest of the reveal path so a mid-flight consent revoke stays silent.
…forever (#371) (#372) * fix(iris-chat): stop a stale selection from pinning the chat context forever (#371) Opening a workspace for exercise "Graph Traversal" showed the chat bound to "Struggle Test Course". Workspace detection had run and succeeded; the override was simply refused. The active context is persisted together with its `source`, and `source === 'user-selected'` was treated as an absolute veto over workspace detection. A selection made three days and several windows earlier therefore came back as an explicit user choice on every start and pinned the chat, no matter which exercise was actually open. The evidence: detection logged its hit, `setActiveContext` never logged at startup (so the context came from persistence), and the override's own log line never appeared. The veto is right within a session — background re-detection must not yank the chat away while the student works — and wrong across one. It is now scoped by an in-memory marker on the manager, set whenever a selection is made through the picker. A context restored from persistence carries `user-selected` but no marker, so detection may take it over exactly once, on startup. The marker is deliberately not a `selectedAt` comparison against activation time: both are wall-clock, so a backward clock step could void a selection the student had just made, which is worse than the bug being fixed. It is armed at the top of `handleContextSelection`, before the same-context early return, since confirming an already-active restored context by clicking its row is a real choice even though nothing else has to happen. Also fixes a latent defect in the same guard: it compared ids without comparing type, so an active COURSE with id N suppressed the override of exercise N. `_autoSelectFromSnapshot` now logs its pick. It was the only path that chooses a context with no explicit signal, and its silence made the log unable to distinguish "auto-picked something" from "chose nothing at all" while this bug was being tracked down. * fix(iris-chat): drop the unused export on the workspace-override policy (#371) It was exported to allow a direct unit test, but the tests ended up going through the manager's public API instead, which is the better level anyway. knip flagged it as an unused export.
This was referenced Aug 9, 2026
The branch forked before #356, #362 and #375 replaced the Iris chat's local-session architecture with the server conversation model. git reported 46 conflicts; the real work was deciding what the branch's code should become, not which side of a hunk to keep. Ownership. The five old source files dev deleted stay deleted, and so do their six tests: chatSessionService, chatContextManager, chatMessageService, sessionManager and contextStore belong to the replaced architecture. Most of the 274 lines the branch had added to them were workarounds for problems #375 solved properly (awaitable session switching, local-id preservation, keeping workspace detection from retargeting the chat) and are gone rather than ported. The three telemetry files the branch deleted stay deleted; dev's one new call site, endTelemetrySession, now reaches StruggleCoordinator, which already had endExerciseSession. Three behaviours did need re-homing. classifyIrisCourseAvailability became a module-level helper in irisAvailabilityService returning { availability, settings }, so the chat provider, the proactive-control commands and IrisEnabledCache share one implementation. Proactive metadata and navigation moved onto ConversationState and conversationService.navigateTo. Three hazards that a clean textual merge would have hidden: - courseIdResolver was not flagged as conflicted and ended up with two declarations of the same function, ours taking the deleted ContextStore. Removed. - currentNavToken mapped to the wrong counter. navigationGeneration only advances once a conversation installs; the reveal guard needs the service's navigation request sequence, which advances when the navigation starts. A navigation in flight must already invalidate an older reveal, so IrisConversationService now exposes navigationRequestToken. - _applyActiveSurface posted the proactive bubble before opening the target conversation, which was only safe while the provider attributed bubbles to a local session. It navigates first now, and openSession carries courseId because nothing established that a proactive session id is globally unique. Verification: check-types, lint and knip clean; 2099 webview tests and 1422 host tests pass; both VSIX variants build, including the clean-bundle verifier.
Both are the kind that fail silently: the code keeps compiling and the suite keeps passing while the behaviour is gone. The stale guard now reads the service's navigation request token rather than ConversationState.navigationGeneration. The two advance at different moments, and only the request sequence moves when a navigation is admitted but its detail request has not returned. A test pins exactly that: a navigation in flight must already invalidate a reveal armed before it. The active surface opens the target conversation before posting the proactive bubble. Under the old local-session model the order did not matter; under the conversation model a bubble posted first is attributed to whichever conversation is still installed. One test holds the open pending and asserts no bubble yet, then releases it and asserts the bubble; a second asserts a failed open still posts, so a network error cannot swallow the hint. Each assertion was mutation-checked. Removing the stale guard fails the two guard tests, hard-coding the course out of navigateTo fails the course-scoping test, awaiting the open fails the focus test, and restoring the old post-then-open order fails the ordering test.
… run `artemis.forceStruggleIntervention` and `artemis.toggleStruggleWarmupSkip` are registered only in `telemetry/index.ts`, which the Open VSX build aliases away to `noop.ts`. The clean manifest dropped `artemis.showStruggleScore` but not these two, so with `artemis.developerMode` on they appeared in the EduIDE palette and failed with "command not found" -- exactly what the drop lists exist to prevent. While checking the drop lists, the verifier turned out to be blind to its own seam entry: `src/extension/telemetry/index.ts` matched no forbidden prefix (`TELEMETRY_SUBTREE` points at the `services/telemetry/` layout this branch does not use), so an import bypassing the `@telemetry` alias would have pulled the engine wiring back into the clean bundle undetected. Forbid it for Open VSX; the Desktop build keeps the file and is unaffected. The manifest and the code it describes drifted because nothing compared them. `verify-clean-bundle.js` proves the excluded code is absent from the BUNDLE; the new test does the same for the MANIFEST, classifying every source file with the verifier's own predicate (now exported as `isForbiddenInput`) rather than keeping a second copy of the exclusion rules. It deliberately does not scan for `registerCommand` call sites -- registrations go through constants too -- and instead fails when a shipped command's id occurs only in dropped code. That is a necessary condition, not a proof that a handler exists, and it is documented as such. Never released: neither command exists on dev.
The repository map still described the pre-v3 tree: `telemetry/` as "struggle detection & recording pipeline" (it is now just the build seam), a `services/telemetry` that does not exist, and `test/` as unit + react when there are six suites. Replace the guesswork with tables for `services/` and the test directories, and note the seam aliases that decide what each variant bundles. Also document the clean manifest properly. It did not just under-describe the step, it was wrong about it: the Open VSX profile overrides three setting defaults (ADR 002) and deletes the prepublish hook, neither of which was mentioned, while the text claimed removals were the only profile-specific change. Record the invariant the drop lists carry and the test that guards it, and record the one contribution that still escapes it: `artemis.iris.proactiveCodeEgress` ships to Open VSX although the code acting on it does not. Smaller corrections: the scripts table was missing six scripts and understated `test:react`, `package:rec` is refused under CI rather than merely unshipped, and golden-replay only runs with the study dataset present.
`.git/info/exclude` already held the specs and plans back, with the reason written next to the rule: they added ~10k lines to a PR diff. But exclude only applies to untracked paths, so two files that predated the rule stayed tracked and rode along into dev and main. Move the rule to `.gitignore`, where it is shared and where an already-tracked file is visible as a contradiction, and delete the two. These are working notes for one change. They describe intent before the code exists and stop matching it the moment it lands, so a reader who trusts them is worse off than one who reads the code. Rationale meant to outlive a pull request belongs in the code, in an ADR, or on the issue -- which is where the parts still worth keeping already are. `extension/docs/plans/` is the same artifact in a third place: the two plans there produced ADRs 002 and 003, which is exactly the outliving form. DEVELOPER.md drops `docs/` from the repository map, since nothing under it is tracked any more, and says where design notes do and do not live.
Brings in the comment cleanup (#418). That commit changes comments only, so conflicts are resolved in favour of this branch: the incoming side carries no executable code and the branch's own version always wins on contact. # Conflicts: # extension/src/extension/services/telemetry/buildResultTracker.ts # extension/src/extension/services/telemetry/debugDashboard.ts # extension/src/extension/services/telemetry/decision/interventionDecisionEngine.ts # extension/src/extension/services/telemetry/diagnosticPersistenceService.ts # extension/src/extension/services/telemetry/eventPipeline/boundaryTriggerEmitter.ts # extension/src/extension/services/telemetry/eventPipeline/compileEquivalentEmitter.ts # extension/src/extension/services/telemetry/eventPipeline/lintDenylist.ts # extension/src/extension/services/telemetry/iTelemetryManager.ts # extension/src/extension/services/telemetry/inactivityService.ts # extension/src/extension/services/telemetry/index.ts # extension/src/extension/services/telemetry/intervention/adaptiveCadence.ts # extension/src/extension/services/telemetry/interventionFilter.ts # extension/src/extension/services/telemetry/interventionService.ts # extension/src/extension/services/telemetry/metrics/buildErrorFamily.ts # extension/src/extension/services/telemetry/metrics/errorQuotientEngine.ts # extension/src/extension/services/telemetry/metrics/snapshotDedup.ts # extension/src/extension/services/telemetry/replay/replayCommand.ts # extension/src/extension/services/telemetry/replay/replayEngine.ts # extension/src/extension/services/telemetry/replay/snapshotReconstructor.ts # extension/src/extension/services/telemetry/telemetryManager.ts # extension/src/extension/services/telemetry/types.ts # extension/test/logic/telemetry/buildFamilyConsistency.test.ts # extension/test/react/services/replay/replayEngine.test.ts # extension/test/unit/services/telemetry/interventionService.test.ts # extension/test/unit/services/telemetry/telemetryManagerCadenceFilter.test.ts # extension/test/unit/services/telemetry/telemetryManagerInterventionToggle.test.ts # extension/test/unit/struggle-detection/EvaluationEngine.ts # extension/test/unit/struggle-detection/ReportGenerator.ts # extension/test/unit/struggle-detection/ScenarioLoader.ts # extension/test/unit/struggle-detection/StruggleTestRunner.ts # extension/test/unit/struggle-detection/boundaryTriggerAndCadence.test.ts # extension/test/unit/struggle-detection/classifyBuildResult.test.ts # extension/test/unit/struggle-detection/errorQuotientEngine.test.ts # extension/test/unit/struggle-detection/index.ts # extension/test/unit/struggle-detection/struggleDetection.test.ts # extension/test/unit/struggle-detection/telemetryManagerCrossExercise.test.ts # extension/test/unit/struggle-detection/types.ts
Applies the same sweep dev received to the files the merge did not reach: the struggle-intervention service and its tests, the telemetry contract, the sensing and struggle trees, and the recording viewer. Removed comments narrating past changes, comments restating the line below them, JSDoc that only re-spelled the signature, banner decoration, and comments that contradicted the code. Where a historical note carried a constraint that still holds, the constraint was rewritten in the present tense rather than deleted. The research record was treated as off limits: provenance markers ([D], [L], [ENG]), SPEC and TUNING labels, paper citations, and any justification of a threshold, correlation or golden-pinned value stay exactly as they were. config.ts was handled as read-only. Spec section references (§5.5, §6, §7.2, §8, §12.2, §13) are preserved; §12.2 on the proactive-control contract was restored after the first pass dropped it. No executable code changes. Verified by comparing re-printed comment-free ASTs for every changed file. check-types, eslint, 2104 extension vitest tests, 1427 mocha tests and 338 recording-viewer tests pass.
The 'struggle' label still pointed at out/test/unit/struggle-detection/**, a directory this branch deleted when the v1 decision path went away. Nothing invokes it: the matching `test:struggle` script is already gone from package.json and no workflow references the label, so it survived only as a config entry that silently selects zero tests. The engine is still covered. Its mocha tests live under test/unit/services/struggle/** and run through the 'unit' label; the rest are vitest suites under test/logic/struggle/** and run through `npm run test:react`. The 'unit' label's comment now says that outright instead of referring to a label that no longer exists. Unit suite unchanged at 1427 tests, 0 failures.
Brings in the OIDC login (#420 with the review fixes from #426), the callback path hardening (#427), the extension host test job (#425) and the Get Started walkthrough (#423). Ten conflicts. Most were both sides adding to the same place and were resolved as the union: the provider and deps imports, the API and manifest test suites, the manifest script header (this branch's ADR 003 reference plus dev's walkthrough group), and the package.json scripts. Two needed a decision rather than a merge: - dev re-adds the 'struggle' vscode-test label pointing at out/test/unit/struggle-detection/**. This branch deleted that directory with the v1 decision path and removed the label in cf1985b precisely because it matched nothing. Taking dev's side would have undone that, so the label stays gone and this branch's explanatory comment is kept over dev's. - test/unit/struggle-detection/README.md was deleted here in e97d082, when the v1 test framework it documented went away, and modified on dev. The deletion stands: dev's edit only corrected commands inside a document this branch has deliberately removed. The docs tables follow from those two: test:unit is renamed to test:vscode as the command CI runs, and test:struggle is deliberately not listed because it does not exist on this branch. The third ArtemisWebviewProvider fixture needed the new oidcLoginService dependency, which the other two had already picked up from dev.
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
1 participant
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.
Summary
Client side of the proactive struggle-intervention feature, on the reworked v3 detection engine. The extension detects struggle locally, requests an Iris intervention from Artemis, and surfaces it non-intrusively, with student controls and an honest availability state.
This is the integration branch for the whole v3 + proactive-intervention client work (engine rework plus the intervention surfaces/controls). Opening as draft for review/visibility.
Note: this PR supersedes #333, which was squash-merged into
devprematurely and reverted there (3e322890); the branch and its history are unchanged.What's included
@telemetryclean-build seam.activeinterventions.courseDisabledflag (course-level proactive off).Proactivity is one level, not a switch
The student control is a three-way Off / Less / More selector (
ProactiveLevel, issue #341), remembered per exercise, defaulting tomore. A legacy storedfalsemaps tooff. It is a single monotone "Iris presence" axis: the detector fires identically at every level,thetais untouched, and only delivery changes.The level is also sent to Artemis as
proactivityModeon the intervention POST, so the server can forceactive→ambientin Pull rather than trusting the client, and Pyris receives it as prompt tone context. The client re-routing instruggleInterventionService.onServerActiveis defence in depth, not the only guard.Delivery is throttled per level (
THROTTLE_BY_LEVEL, read live on every delivery):At More the SPEC cooldown (120 s) is the only spacing, so the throttle contributes only the session budget. At Less the 600 s gap genuinely bites. The two layers deliberately do not overlap.
Cross-repo
Pairs with ls1intum/Artemis#13023 (server) and ls1intum/edutelligence#756 (Pyris pipeline).
Notes
backoffGateis still present and wired intostruggleInterventionService. Retiring the cumulative dismiss memory in favour of the per-level throttle above is pending on this branch.scripts/verify-clean-bundle.js).check-types+ eslint + clean-bundle green.