Skip to content

Add per-root Code Map generation controls#609

Open
provencher wants to merge 1 commit into
perf/swift-pipeline-deep-optimizationfrom
feature/codemap-root-control
Open

Add per-root Code Map generation controls#609
provencher wants to merge 1 commit into
perf/swift-pipeline-deep-optimizationfrom
feature/codemap-root-control

Conversation

@provencher

Copy link
Copy Markdown
Collaborator

Summary

  • publish event-driven per-root Code Map status/progress into Agent Mode workspace rows
  • add compact pause/resume controls with pending-action and accessibility states
  • gate preload, retry, invalidation, setup, and explicit-demand paths while a loaded root is paused

Pause semantics

Pausing is scoped to the current loaded root lifetime. It immediately revokes publication authority, cancels queued/retry/setup/demand work, and prevents new generation admission. Already-admitted engine transactions may drain to the existing safe currentness boundary but cannot publish as current. Resuming waits for cleanup and schedules one fresh preload; unloading or restarting resets the pause.

Validation

  • make dev-test FILTER=AgentWorkspaceRootsSidebarStoreTests — passed, 24 tests
  • make dev-test FILTER=CodemapPreloadTests — passed, 16 tests
  • make dev-test FILTER=CodemapBindingEngineProjectionTests — passed, 16 tests
  • make dev-test-list — passed
  • python3 Scripts/test_suite_optimizer.py verify-ledger --ledger Scripts/Fixtures/test-suite-contract-ledger.tsv — expected unrelated repository mismatch remains: 5 missing IDs, 0 stale; the three feature rows are present
  • contribution commit preflight — passed
  • contribution push preflight — passed against the one-commit stacked range
  • PR-ready lint — passed (0/1384 files require formatting; SwiftLint strict passed)
  • PR-ready full root tests — blocked by existing CodexNativeSessionControllerGoalConfigTests.testAgentModeDefaultCarriesGoalFeatureConfigToStartAndResume goal-default assertions; the exact test reproduces on the untouched stacked base
  • make dev-swift-build PRODUCT=RepoPrompt — passed
  • live smoke not run because this change was prepared without launching/relaunching the app

Stack note

This PR intentionally targets perf/swift-pipeline-deep-optimization (the retained remote branch for merged PR #608). The hosted comparison is exactly one commit and the 11 codemap feature/test/ledger files, avoiding unrelated history from a direct branch-base comparison workflow. Retarget to main when appropriate after review.

@baron baron left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Request changes — production design looks solid; two pre-merge items.

1. Confirmed flaky test (hosted shard 3). Tests/RepoPromptTests/WorkspaceContext/CodemapBindingEngineProjectionTests.swift:117 asserts observedStatuses.first == .unavailable(.jobNotScheduled), but projectionSnapshotUpdates intentionally uses .bufferingNewest(1) and the collector Task is unsynchronized. The initial value is buffered at subscription; if scheduleProjectionPreload publishes before the collector’s first next(), newest-1 drops that value and the first observation is .pending(.scheduled) — exactly what shard 3 reported. The contains { .pending } assertion has the same exposure if the collector wakes only after completion. Please make observation deterministic by consuming/acknowledging the initial value before scheduling, or assert only guarantees of a newest-1 stream (eventual .authoritativeComplete and no consecutive duplicates). Keeping newest-1 in production is correct; this is a test-only fix.

2. Retarget to main before merge. The stacked base perf/swift-pipeline-deep-optimization served its review purpose, but merging as-is lands on that retained feature branch rather than main. PR #608’s head is already an ancestor of main, so retargeting should preserve the identical one-commit/11-file content diff. Require green hosted checks on the retargeted merge ref.

A second independent review confirmed the production path is coherent: suspension is installed before detachment, publication authority is revoked synchronously, cleanup is awaited before resume, re-pause cancels an in-flight resume through the transition token, admission is gated across authority/preload/retry/setup/explicit and multi-ticket demand paths, and shutdown/detach/unload clean up the new streams and observer task.

Non-blocking notes only: pause is intentionally in-memory for the loaded-root lifetime; cancelled/superseded idle states can temporarily render MAP …; and rowHasContextLine is now unconditionally true and could be simplified later.

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.

2 participants