Skip to content

Remove obsolete Builder, Debugger, and Generalist definitions - #97

Merged
loganj merged 1 commit into
mainfrom
other-brother-darryl/remove-recommended-agents
Aug 20, 2026
Merged

Remove obsolete Builder, Debugger, and Generalist definitions#97
loganj merged 1 commit into
mainfrom
other-brother-darryl/remove-recommended-agents

Conversation

@loganj

@loganj loganj commented Aug 19, 2026

Copy link
Copy Markdown
Collaborator

🤖 Brother Darryl

Summary

Remove Builder, Debugger, and Generalist from Berd’s retired onboarding recommendation catalog while preserving valid version-1 onboarding records that selected one of those retired IDs.

Details

  • Deletes only the three obsolete recommendation definitions and their English/Spanish onboarding copy.
  • During version-1 hydration and established-install startup graduation, normalizes only builder, debugger, and generalist selections to null before existing validation. Lifecycle, consent, provider/harness setup, work types, and all unrelated valid fields remain intact.
  • Unknown agent IDs remain invalid and continue to reset the malformed record.
  • Bundled agents, avatars, and current visible onboarding behavior are unchanged.

Verification

  • Blox full onboarding suite: pnpm vitest run src/features/onboarding — 13 files, 80 passed at 598957205be228617ceaac95bf0b749f5600132a.
  • Direct-hydration discriminator covers all three retired IDs.
  • Startup-order discriminator covers all three retired IDs across in-progress/recommendations and completed/complete lifecycle states with work types, harness/setup, and consent populated. It fails at 5d4aac1 and passes at the final head.

@loganj
loganj marked this pull request as ready for review August 19, 2026 02:12
@loganj
loganj requested a review from a team August 19, 2026 02:12
@loganj
loganj marked this pull request as draft August 19, 2026 02:13
@loganj
loganj marked this pull request as ready for review August 19, 2026 11:29
@loganj loganj changed the title Remove obsolete onboarding recommendations Remove three onboarding recommendations and Agt. Builder Aug 19, 2026
@loganj loganj changed the title Remove three onboarding recommendations and Agt. Builder Remove three obsolete onboarding recommendations Aug 19, 2026
@loganj
loganj marked this pull request as draft August 19, 2026 13:01
@loganj
loganj force-pushed the other-brother-darryl/remove-recommended-agents branch from 4992660 to 0cec887 Compare August 19, 2026 13:08
@loganj loganj changed the title Remove three obsolete onboarding recommendations Stop recommending Builder, Debugger, and Generalist during setup Aug 19, 2026
@loganj loganj changed the title Stop recommending Builder, Debugger, and Generalist during setup Remove obsolete Builder, Debugger, and Generalist definitions Aug 19, 2026
@loganj
loganj force-pushed the other-brother-darryl/remove-recommended-agents branch from 0cec887 to f6c59d0 Compare August 19, 2026 13:36
@loganj
loganj marked this pull request as ready for review August 19, 2026 14:32

@morgmart morgmart 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.

🤖 Automated code review

REQUEST_CHANGES: removing catalog IDs that remain valid in version-1 persisted onboarding records causes those records to fail hydration and reset to the initial onboarding state. The supplied GitHub evidence was inspected; all eight supplied check runs for the exact head SHA completed successfully. Static review only; no local code, tests, builds, package managers, hooks, binaries, or project scripts were run.

Deterministic publication result: 1 blocking and 0 non-blocking finding(s) publishable; 0 duplicate(s) suppressed.

Comment thread src/features/onboarding/model/catalog.ts
@loganj
loganj force-pushed the other-brother-darryl/remove-recommended-agents branch 2 times, most recently from f636831 to 87e7779 Compare August 20, 2026 00:53

@morgmart morgmart 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.

🤖 Automated code review

NO PUBLICATION: the fresh full review found that the previously reported persisted-onboarding compatibility issue remains on the established-installation startup path, but the same underlying issue already has an unresolved automated GitHub thread and is therefore suppressed rather than republished. parsePersisted now normalizes retired IDs, but initializeOnboardingGraduation first checks the unnormalized record with isValidPersistedState; for an established installation, that check fails and the function overwrites the record with completedState before hydration, still discarding preserved consent, harness, setup, and work-type fields. The supplied GitHub evidence was inspected: Frontend checks and unit tests failed, three Tauri checks were in progress, two checks were skipped, and two passed at capture time; required checks still govern merge readiness. Static inspection only; no local code, tests, builds, package managers, generated binaries, hooks, or project scripts were run.

Deterministic publication result: 0 blocking and 0 non-blocking finding(s) publishable; 1 duplicate(s) suppressed.

@loganj
loganj force-pushed the other-brother-darryl/remove-recommended-agents branch from 87e7779 to 5d4aac1 Compare August 20, 2026 00:59

@loganj loganj left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

🤖 Carl review at exact head 5d4aac1aea8a624b2c9fd658ac9b9337739b1554.

Blocker — startup graduation still destroys established users’ persisted onboarding outcomes when they selected a retired recommendation.

parsePersisted() now normalizes builder / debugger / generalist before validation (src/features/onboarding/model/onboardingStore.ts:160-188), but normal app startup calls initializeOnboardingGraduation() first (src/main.tsx:190-197). Its preserveExisting branch still passes the raw record directly to isValidPersistedState() (onboardingStore.ts:76-96). Because agentIds no longer contains those three IDs (onboardingStore.ts:39-45), an established install’s otherwise-valid record fails that check. For cohort established-before-landing-v1, lines 96-106 then overwrite the whole record with completedState, discarding selected work types, configured/completed harness setup IDs, and the usage-data consent answer before parsePersisted() can perform the repair.

Affected state: any established-cohort upgrade carrying one of the three retired selectedAgentId values, across completed or in-progress records. Fresh/unknown cohorts reach the new normalization, but established users take the destructive branch. The added parameterized test exercises hydration directly and therefore does not cover the production startup ordering.

Please apply the same retired-ID normalization when deciding whether graduation should preserve a current-version record (or otherwise make the startup preservation check use the repaired state), and add a regression test that calls initializeOnboardingGraduation("established-before-landing-v1") with all durable fields populated before hydrating.

I found no separate product-effect issue in the other seven files: the catalog/i18n removals are confined to the retired recommendations; bundled distro/agents and already-created persona files are separate; the remaining catalog order and three-result limit are deterministic; malformed and unrelated unknown IDs remain rejected. I am not approving while the startup path above remains destructive.

Co-authored-by: Brother Darryl <146fb160a3266e6165bfa385f6048c975eda9e21cf65da097a0b5ea7952532a5@buzz.block.builderlab.xyz>

Signed-off-by: loganj <loganj@squareup.com>
@loganj
loganj force-pushed the other-brother-darryl/remove-recommended-agents branch from 5d4aac1 to 5989572 Compare August 20, 2026 01:03

@loganj loganj left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

🤖 Princess Donut — PASS. I independently audited exact head 598957205be228617ceaac95bf0b749f5600132a on a fresh read-only Blox workstation. The removal is confined to the retired recommendation catalog/copy; both persisted-state entry paths normalize only builder, debugger, and generalist before the existing validator, preserving lifecycle, work types, harness/setup, and consent while unrelated unknown or malformed IDs remain rejected. Agt. Builder and existing bundled/created personas are outside the changed catalog. English/Spanish keys match the four surviving entries, recommendation output remains three entries for every work type, pnpm check passed, onboarding tests passed 80/80, and full Vitest passed 6,667 with 1 skipped. No unexpected product effects found. I attempted APPROVE, but GitHub rejects approval through Logan’s account as self-approval, so this is a COMMENT review.

@loganj loganj left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

🤖 Carl final audit at exact head 598957205be228617ceaac95bf0b749f5600132a: PASS (COMMENT verdict; GitHub does not allow Logan’s account to approve its own PR).

I re-audited the complete eight-file diff and both persisted-state entry paths on a fresh read-only Blox workstation. The prior blocker is coherently repaired: initializeOnboardingGraduation() and parsePersisted() both apply the same narrow normalization before the existing validator (src/features/onboarding/model/onboardingStore.ts:88-92,162-189). Only builder, debugger, and generalist become null; unrelated unknown IDs and malformed records still take the pre-existing rejection path.

The startup-order regression covers all three retired IDs across valid in-progress/recommendations and completed/complete states while preserving work types, selected/completed harness setup, and consent (onboardingStore.test.ts:80-108). I also traced catalog consumers and confirmed the remaining recommendation order/three-result behavior stays deterministic; locale removals match only the removed entries; bundled distro/agents and already-created persona files are outside this catalog. distro/agents/agt-builder.md remains present with app-avatar:gloopies-3.

Independent Blox validation at this exact clean head:

  • just check: passed, including Biome formatting, i18n, contract freshness, and TypeScript.
  • Full just test: 562 files passed; 6,667 tests passed, 1 skipped.

No blocking or non-blocking findings.

@loganj
loganj dismissed morgmart’s stale review August 20, 2026 01:12

🤖 Leo — The persisted-state finding is addressed at 5989572. Both startup entry paths now apply the narrow retired-ID normalization before validation; parent-red/final-green coverage exercises all three retired IDs across in-progress and completed states while preserving work types, harness setup, and consent. All three independent Royal audits passed the exact final head.

@morgmart morgmart 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.

🤖 Automated code review

APPROVE: the previously reported persisted-onboarding compatibility issue is fixed across both direct hydration and the established-installation graduation path. The shared normalization clears only the three retired recommendation IDs before validation, while preserving lifecycle, work types, harness/setup state, and usage-data consent; focused tests now exercise both in-progress and completed startup records for every retired ID. A fresh full review found no new publishable findings. Final self-check covered the changed onboarding and startup flows, accessibility, English/Spanish localization, navigation and consent preservation, async/error/never-completes/lifecycle/race behavior, test honesty, design-system and project rules, duplicate overlap, and blocking-evidence quality. The supplied GitHub evidence was inspected and all eight check runs for the exact head SHA completed successfully. Static inspection only; no local code, tests, builds, package managers, generated binaries, hooks, or project scripts were run.

Deterministic publication result: 0 blocking and 0 non-blocking finding(s) publishable; 0 duplicate(s) suppressed.

Pending checks: 1 check(s) are not complete.

This approval reflects the completed code review only; merge readiness remains governed by the repository's required checks.

@loganj
loganj merged commit a1a41ce into main Aug 20, 2026
8 checks passed
@loganj
loganj deleted the other-brother-darryl/remove-recommended-agents branch August 20, 2026 02:17
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