fix: repair main — Phases 2 and 3 each shipped their own lifecycle reader#745
Merged
Conversation
…le reader
`main` does not compile:
error[E0432]: unresolved import `tuning::session_lifecycle_enabled`
--> stella-cli/src/memory.rs:78:9
Third instance of the same failure mode this week, and the clearest one yet.
Phase 2 (#738) and Phase 3 (#736) were built in parallel, and each independently
needed to read `context.lifecycle.enabled`. Neither could see the other, so both
wrote the reader — with a byte-identical body and the same fail-closed
reasoning, differing only in name and visibility:
Phase 3: pub(super) fn lifecycle_enabled(..)
Phase 2: pub fn session_lifecycle_enabled(..)
Phase 3 merged first. Phase 2 merged second from a rebase that predated it, so
`tuning.rs` kept Phase 3's function while `memory.rs` kept Phase 2's re-export
of a name that no longer exists. Neither PR was wrong and neither conflicted
textually with what was on main at the moment it merged.
The fix is the resolution both branches were converging on anyway: one reader,
`pub` because Phase 2's engine-config builder calls it from outside the module,
documented with both phases' reasoning, and one call site updated.
Worth stating plainly, because it has now cost three repairs: parallel branches
that merge cleanly are not thereby compatible. A merge that lands second is
verified against the main it was rebased onto, not the main it lands on, and
nothing in the tooling notices the difference. Re-verifying the second branch
against the actual post-merge tree is the step that catches this, and it is
cheap compared to a red main.
`make gate` green: exit 0, 3801 tests, check-file-size OK.
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @macanderson, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
macanderson
added a commit
that referenced
this pull request
Jul 26, 2026
`context.lifecycle.enabled` now defaults to **`true`**. The whole lifecycle — decomposed recall, frame identity, recall telemetry, selection reasons, the typed learning loop, the ledger and its review surface — is what a workspace gets without configuring anything. > Stacked on #745, which repairs a `main` that does not currently compile. Review that first. ## Why this is safe to default on The behavior-compatibility suite asserts **every spec §8 guarantee on both the typed path and the opted-out one**, from the same assertions: tombstone suppression across re-learning on every surface, the per-session creation cap, never clobbering a hand-edited file, deterministic candidate identity, and failure isolation. Phase 3 deliberately kept the lexical loop reachable rather than deleting it, so "behavior-compatible" stays a checkable claim rather than a promise. Defaulting on is the point of having built that evidence — a lifecycle nobody runs cannot be evaluated. ## The flip is two changes, not one This is the part worth reviewing closely. **Flipping `LifecycleSettings::default()` alone would have shipped nothing.** The reader asked: ```rust .and_then(|s| s.context).is_some_and(|c| c.lifecycle.enabled) ``` A workspace with no `settings.json` — the common case, and every new user — answered `false` regardless of the struct's default, because the `context` block was **absent** rather than present-and-false. An absent block now means "the defaults", which is what it always should have meant. There are tests for the no-settings-file case and the empty-block case specifically, because that is exactly the gap that would have shipped this as a silent no-op. ## What still fails closed An **unreadable or malformed** settings file stays OFF. That file may be the one in which someone turned the lifecycle off, and silently overriding an opt-out we failed to parse is worse than leaving a default unapplied. An explicit `"enabled": false` is honored and restores the previous behavior wholesale. ## Notes for the reviewer - The test fixtures that used to get the lexical loop *by writing nothing* now opt out explicitly, since "write nothing and get the old path" is no longer true. That is a change to what the fixtures **say**, not to what they **assert** — no assertion was relaxed. - `DriverConfig::lifecycle_enabled` still defaults `false` on purpose: a programmatically built config should opt in deliberately rather than inherit a product default it never read. The CLI passes the real setting. - `learning.mode` and `governance.mode` remain inert — they have no consumers outside settings tests. `lifecycle.enabled` is the only real switch. ## Gate `make gate` green: **exit 0, 3805 tests**, `check-file-size: OK — 475 Rust files, none over 1500 lines except 25 grandfathered (none grew).`
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.
maindoes not compile.Cause
Phase 2 (#738) and Phase 3 (#736) were built in parallel, and each independently needed to read
context.lifecycle.enabled. Neither could see the other, so both wrote the reader — with a byte-identical body and the same fail-closed reasoning, differing only in name and visibility:pub(super) fn lifecycle_enabled(..)pub fn session_lifecycle_enabled(..)Phase 3 merged first. Phase 2 merged second from a rebase that predated it, so
tuning.rskept Phase 3's function whilememory.rskept Phase 2's re-export of a name that no longer exists.Neither PR was wrong, and neither conflicted textually with the
mainthat existed when it merged.Fix
The resolution both branches were converging on anyway: one reader,
pubbecause Phase 2's engine-config builder calls it from outside the module, documented with both phases' reasoning, and one call site updated.The pattern, since this is the third repair this week
MemoryCmd; The three #729 follow-ups: compaction, an ANN index, and ADR 0006's amendment #735 was cut from an older main and re-added it. Zero textual conflict, nine type errors.Parallel branches that merge cleanly are not thereby compatible. A branch that lands second is verified against the main it was rebased onto, not the main it lands on, and nothing in the tooling notices the difference. Re-verifying the second branch against the actual post-merge tree is the step that catches this, and it is far cheaper than a red main.
Gate
make gategreen: exit 0, 3801 tests,check-file-size: OK — 475 Rust files, none over 1500 lines except 25 grandfathered (none grew).