Generate context files when they already exist in the LST#16
Merged
Conversation
When the build-time LST manifest includes existing .moderne/context/* files
(as it does in moderne-cli 4.1.4+ after PR #3661), the recipes in this module
that defer work to cycle 2 stop firing for repos that already have those
files committed. The result is that running UpdatePrethinkContextNoAiStarter
across a multi-repo corpus generates context for some repos and silently
skips others.
ExportContext.getVisitor returns the tree unchanged on cycle 1 because data
tables are populated during the visitor phase of cycle 1 and read in
cycle 2. ExportContext.generate also defers to cycle 2 for the same reason.
The recipe contributes zero changes in cycle 1, and the only way it ever
got to cycle 2 was that some other recipe in the pipeline (typically
GenerateCalmArchitecture's placeholder, which writes a `{}` file at
.moderne/context/calm-architecture.json) made a cycle-1 change and kept the
RecipeScheduler loop alive. Once the placeholder workaround stops firing
(because calm-architecture.json is now present in the accumulator from the
scanner), the loop terminates after cycle 1 and ExportContext never runs.
This change makes the cycle-1 branch of ExportContext.getVisitor call
ctx.putMessage with an "io.moderne.prethink." key. RecipeRunCycle treats any
ExecutionContext mutation from a recipe as "this recipe made a change", and
combined with the existing causesAnotherCycle override it enrolls the recipe
for cycle 2. The CursorValidatingExecutionContextView allow-list requires
the io.moderne. prefix.
GenerateCalmArchitecture has the same shape: its cycle-1 placeholder
workaround in generate() only fires when calm-architecture.json is absent
from the accumulator, and its visitor has a "cycle != 2" early return. When
the file is already in the LST, the placeholder is skipped and the visitor
never runs in cycle 1 to keep the loop alive. The same fix is applied to
its visitor.
Both fixes are defense-in-depth in pipelines where other recipes already
trigger cycle 2. They become load-bearing when this module's recipes are
used standalone or in a pipeline whose other members don't make cycle-1
changes.
Regression tests added to ExportContextTest and GenerateCalmArchitectureTest
invoke Recipe.run directly with maxCycles=3, minCycles=1, matching the
production CLI's RecipeScheduler.scheduleRun call. RewriteTest.rewriteRun
cannot represent this scenario because it strictly enforces
expectedCyclesThatMakeChanges, and the only way to set minCycles=1 through
that API also asserts zero cycles make changes -- contradicting the success
criterion. Both tests fail without their respective fixes.
Both ExportContext and GenerateCalmArchitecture set the same kind of marker on the ExecutionContext to request another scheduler cycle. Each was using its own hardcoded string literal, which made the shared intent invisible at the call site and risked drift if either was edited. Move the key to Prethink.CYCLE_TRIGGER alongside the existing CONTEXT_DIR constant. The Javadoc on the constant explains the side-effect-only semantics (the key is never read; ctx.putMessage flips WatchableExecutionContext.hasNewMessages, which RecipeRunCycle treats as the recipe contributing a change for cycle-2 enrollment) and notes that the io.moderne. prefix is required by CursorValidatingExecutionContextView's allow-list.
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
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.
Background / problem
.moderne/context/*files (as it does in moderne-cli 4.1.4+ after moderne-cli#3661), the recipes in this module that defer work to cycle 2 stop firing for repos that already have those files committed. The customer-facing symptom is that runningUpdatePrethinkContextNoAiStarteracross a multi-repo corpus generates context for some repos and silently skips others.ExportContext.getVisitorreturns the tree unchanged on cycle 1 because data tables are populated during the visitor phase of cycle 1 and read in cycle 2.ExportContext.generatealso defers to cycle 2 for the same reason. The recipe contributes zero changes in cycle 1, and the only thing that historically kept it cycling was that some other recipe in the pipeline (typicallyGenerateCalmArchitecture's placeholder, which writes a{}file at.moderne/context/calm-architecture.json) made a cycle-1 change and kept theRecipeSchedulerloop alive. Once that placeholder workaround stops firing -- becausecalm-architecture.jsonis now present in the scanner's accumulator -- the loop terminates after cycle 1 andExportContextnever runs.GenerateCalmArchitecturehas the same shape: its cycle-1 placeholder only fires when the file is absent, and its visitor has acycle != 2early return.Solution
Both recipes' cycle-1 visitor branches now call
ctx.putMessagewith anio.moderne.prethink.*key.RecipeRunCycletreats anyExecutionContextmutation from a recipe as "this recipe made a change" and, combined with the existingcausesAnotherCycleoverride, enrolls the recipe for cycle 2. TheCursorValidatingExecutionContextViewallow-list requires theio.moderne.prefix. The fixes are defense-in-depth in pipelines where other recipes already trigger cycle 2, and become load-bearing when these recipes are used standalone or alongside other recipes that don't produce cycle-1 changes.Test plan
ExportContextTestandGenerateCalmArchitectureTestthat invokeRecipe.rundirectly withmaxCycles=3, minCycles=1, matching the production CLI'sRecipeScheduler.scheduleRuncall.RewriteTest.rewriteRuncannot represent this scenario because it strictly enforcesexpectedCyclesThatMakeChanges, and the only way to setminCycles=1through that API also asserts zero cycles make changes. Both tests fail without their respective fixes.moderneincrepos): per-repo.moderne/context/*entries infix.patchgo from21 + 0 + 0(broken) to21 + 16 + 18(each repo now produces output). The numeric delta vs CLI 4.1.2's28 + 24 + 21is byte-identical existing on-disk content (verified with a per-file diff) -- the patch is minimal-diff while the pre-#3661 codepath emits "create new file" entries even when content matches.28 + 24 + 21.Relationship to #17
#17 is an alternative that fixes the same bug and introduces a
PrethinkContextRecipebase class, migratingExportContextandGenerateCalmArchitectureto extend it. Either PR can ship on its own. Pick one.