From f5ddfc343d98e2b7cfab7f72d2b55fa68ecfde68 Mon Sep 17 00:00:00 2001 From: Pol Michel Date: Tue, 28 Jul 2026 09:58:09 +0200 Subject: [PATCH 01/16] refactor(regeneration): give selectors their workflow, cascade role, output, and consolidation Each definition selector now owns the kind-specific facts the follow-up used to reach into: the workflow that runs its requests, a CascadeRole (a SOURCE produces output the cascade re-reads; a TERMINAL ends the chain), how to capture its own output, and how to consolidate its requests. A cascade source returns a CascadeSourceOutput binding the capturer to the generators it selected; a terminal selector merges requests for the same definition. The plan becomes a list of entries, one per selector, pairing its requests with that workflow, role, and output. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01B7rfYnzxsPEm45r3e2NnLm --- .../definition_selector/artifact_selector.py | 31 ++++++++++- .../definition_selector/base.py | 25 ++++++++- .../definition_selector/generator_selector.py | 35 +++++++++++- .../generator_diff_capturer.py | 15 ++++++ .../core/merge/selective_regen/models.py | 54 +++++++++++++++++-- 5 files changed, 153 insertions(+), 7 deletions(-) diff --git a/backend/infrahub/core/merge/selective_regen/definition_selector/artifact_selector.py b/backend/infrahub/core/merge/selective_regen/definition_selector/artifact_selector.py index 3a17c357f9..d96fb63746 100644 --- a/backend/infrahub/core/merge/selective_regen/definition_selector/artifact_selector.py +++ b/backend/infrahub/core/merge/selective_regen/definition_selector/artifact_selector.py @@ -1,5 +1,7 @@ from __future__ import annotations +from typing import TYPE_CHECKING + from infrahub_sdk.protocols import CoreArtifactDefinition from infrahub.core.constants import InfrahubKind @@ -8,15 +10,42 @@ from infrahub.git.models import RequestArtifactDefinitionGenerate from infrahub.git.utils import fetch_artifact_definition_targets from infrahub.message_bus.types import ProposedChangeArtifactDefinition +from infrahub.workflows.catalogue import REQUEST_ARTIFACT_DEFINITION_GENERATE -from ..models import LoadedDefinition +from ..models import CascadeRole, LoadedDefinition from .base import DefinitionSelectorBase +if TYPE_CHECKING: + from collections.abc import Sequence + class ArtifactSelector(DefinitionSelectorBase[ProposedChangeArtifactDefinition, RequestArtifactDefinitionGenerate]): """Selects the artifact definitions a merge changed, narrowed to the members it affects.""" subscriber_kind = InfrahubKind.ARTIFACT + workflow = REQUEST_ARTIFACT_DEFINITION_GENERATE + cascade_role = CascadeRole.TERMINAL + + def consolidate( + self, requests: Sequence[RequestArtifactDefinitionGenerate] + ) -> list[RequestArtifactDefinitionGenerate]: + """Merge requests for the same artifact definition, unioning their member/limit filters. + + An artifact selected from both the merge diff and a generator's output would otherwise be + dispatched twice; an empty filter means "all members", so it subsumes any specific filter. + """ + consolidated: dict[str, RequestArtifactDefinitionGenerate] = {} + for request in requests: + merged = consolidated.get(request.artifact_definition_id) + if merged is None: + consolidated[request.artifact_definition_id] = request + continue + members = [] if not merged.members or not request.members else sorted({*merged.members, *request.members}) + limit = [] if not merged.limit or not request.limit else sorted({*merged.limit, *request.limit}) + consolidated[request.artifact_definition_id] = merged.model_copy( + update={"members": members, "limit": limit} + ) + return list(consolidated.values()) async def load_definitions(self, *, target_branch: str) -> list[LoadedDefinition[ProposedChangeArtifactDefinition]]: definition_information = await self.client.execute_graphql( diff --git a/backend/infrahub/core/merge/selective_regen/definition_selector/base.py b/backend/infrahub/core/merge/selective_regen/definition_selector/base.py index 7eda702284..d06d5d4d00 100644 --- a/backend/infrahub/core/merge/selective_regen/definition_selector/base.py +++ b/backend/infrahub/core/merge/selective_regen/definition_selector/base.py @@ -10,15 +10,17 @@ if TYPE_CHECKING: import logging + from collections.abc import Sequence from infrahub_sdk.client import InfrahubClient from infrahub_sdk.diff import NodeDiff from infrahub.core.regeneration.models import RegenerationTrigger + from infrahub.workflows.models import WorkflowDefinition from ..gate import DefinitionGate from ..impacted import ImpactedSubscriberResolver - from ..models import LoadedDefinition + from ..models import CascadeRole, CascadeSourceOutput, LoadedDefinition def _narrow_members_filter(rendered_members: list[str], total_members: int) -> list[str]: @@ -42,6 +44,10 @@ class DefinitionSelectorBase[DefinitionT: DefinitionModel, RequestT](ABC): """ subscriber_kind: str + workflow: WorkflowDefinition + """The workflow that runs this selector's requests.""" + cascade_role: CascadeRole + """This selector's role in the merge regeneration cascade, which orders how it runs.""" def __init__( self, @@ -132,6 +138,23 @@ async def select( requests.append(self._build_request(definition=definition, target_branch=target_branch, members=members)) return requests + def output_capture(self, requests: Sequence[RequestT]) -> CascadeSourceOutput | None: # noqa: ARG002 + """Return how to capture this selector's output for the cascade, or None if it is not a source. + + Overridden by cascade-source selectors, which know their concrete request type and so can build + the capture without the follow-up narrowing it. + """ + return None + + def consolidate(self, requests: Sequence[RequestT]) -> Sequence[RequestT]: + """Combine this selector's requests before dispatch; by default each is dispatched as-is. + + Overridden where several requests can target the same definition (an artifact selected by both + the merge diff and a generator's output), so the follow-up submits one request per definition + without knowing how to merge them. + """ + return requests + async def _map_subscribers_by_member(self, *, definition: DefinitionT, target_branch: str) -> dict[str, str]: """Map each member to the id of its existing subscriber for this definition on the branch. diff --git a/backend/infrahub/core/merge/selective_regen/definition_selector/generator_selector.py b/backend/infrahub/core/merge/selective_regen/definition_selector/generator_selector.py index b9c694cde3..19ce2e92c5 100644 --- a/backend/infrahub/core/merge/selective_regen/definition_selector/generator_selector.py +++ b/backend/infrahub/core/merge/selective_regen/definition_selector/generator_selector.py @@ -1,5 +1,7 @@ from __future__ import annotations +from typing import TYPE_CHECKING + from infrahub_sdk.protocols import CoreGeneratorDefinition from infrahub.core.constants import InfrahubKind @@ -10,15 +12,46 @@ build_generator_definition, ) from infrahub.git.utils import fetch_proposed_change_generator_definition_targets +from infrahub.workflows.catalogue import REQUEST_GENERATOR_DEFINITION_RUN -from ..models import LoadedDefinition +from ..generator_diff_capturer import GeneratorTrackingOutput +from ..models import CascadeRole, CascadeSourceOutput, LoadedDefinition from .base import DefinitionSelectorBase +if TYPE_CHECKING: + import logging + from collections.abc import Sequence + + from infrahub_sdk.client import InfrahubClient + + from ..gate import DefinitionGate + from ..generator_diff_capturer import GeneratorMutationDiffCapturer + from ..impacted import ImpactedSubscriberResolver + class GeneratorSelector(DefinitionSelectorBase[ProposedChangeGeneratorDefinition, RequestGeneratorDefinitionRun]): """Selects the generator definitions flagged to execute after a merge, narrowed to affected members.""" subscriber_kind = InfrahubKind.GENERATORINSTANCE + workflow = REQUEST_GENERATOR_DEFINITION_RUN + cascade_role = CascadeRole.SOURCE + + def __init__( + self, + client: InfrahubClient, + gate: DefinitionGate, + impacted_resolver: ImpactedSubscriberResolver, + log: logging.Logger | logging.LoggerAdapter[logging.Logger], + output_capturer: GeneratorMutationDiffCapturer, + ) -> None: + super().__init__(client=client, gate=gate, impacted_resolver=impacted_resolver, log=log) + self._output_capturer = output_capturer + + def output_capture(self, requests: Sequence[RequestGeneratorDefinitionRun]) -> CascadeSourceOutput: + return GeneratorTrackingOutput( + capturer=self._output_capturer, + definition_names=[run.generator_definition.definition_name for run in requests], + ) async def load_definitions( self, *, target_branch: str diff --git a/backend/infrahub/core/merge/selective_regen/generator_diff_capturer.py b/backend/infrahub/core/merge/selective_regen/generator_diff_capturer.py index eb1caf9194..a3c136d813 100644 --- a/backend/infrahub/core/merge/selective_regen/generator_diff_capturer.py +++ b/backend/infrahub/core/merge/selective_regen/generator_diff_capturer.py @@ -37,6 +37,21 @@ class GeneratorMutationDiffCapturer(Protocol): async def capture(self, *, since: Timestamp, generator_definition_names: list[str]) -> list[NodeDiff]: ... +class GeneratorTrackingOutput: + """The cascade output of a set of generators, captured through their per-member tracking groups. + + Binds the generators' definition names to the capturer so the follow-up can capture their output + without handling the names itself. + """ + + def __init__(self, *, capturer: GeneratorMutationDiffCapturer, definition_names: list[str]) -> None: + self._capturer = capturer + self._definition_names = definition_names + + async def capture(self, *, since: Timestamp) -> list[NodeDiff]: + return await self._capturer.capture(since=since, generator_definition_names=self._definition_names) + + class GeneratorTrackingGroupDiffCapturer: """Capture a post-merge generator's own writes, scoped to the nodes it tracked. diff --git a/backend/infrahub/core/merge/selective_regen/models.py b/backend/infrahub/core/merge/selective_regen/models.py index 18a5c5ba43..2723027a3b 100644 --- a/backend/infrahub/core/merge/selective_regen/models.py +++ b/backend/infrahub/core/merge/selective_regen/models.py @@ -1,17 +1,49 @@ from __future__ import annotations from dataclasses import dataclass -from typing import TYPE_CHECKING +from enum import Enum +from typing import TYPE_CHECKING, Protocol if TYPE_CHECKING: + from collections.abc import Sequence + + from infrahub_sdk.diff import NodeDiff + + from infrahub.core.timestamp import Timestamp from infrahub.generators.models import ProposedChangeGeneratorDefinition, RequestGeneratorDefinitionRun from infrahub.git.models import RequestArtifactDefinitionGenerate from infrahub.message_bus.types import ProposedChangeArtifactDefinition + from infrahub.workflows.models import WorkflowDefinition type DefinitionModel = ProposedChangeGeneratorDefinition | ProposedChangeArtifactDefinition """A definition the merge regeneration selects over: a generator or an artifact definition.""" +type RegenerationRequest = RequestGeneratorDefinitionRun | RequestArtifactDefinitionGenerate +"""A request a selector emits for one definition it decided to regenerate.""" + + +class CascadeRole(Enum): + """A selector's role in the merge regeneration cascade, which orders how the follow-up runs it. + + A ``SOURCE`` produces output that the cascade re-reads, so its runs must complete and have their + writes captured before the terminals are selected. A ``TERMINAL`` is the end of the chain -- nothing + downstream re-reads its output -- so it is dispatched fire-and-forget afterwards. + """ + + SOURCE = "source" + TERMINAL = "terminal" + + +class CascadeSourceOutput(Protocol): + """Captures the diff of what a cascade source wrote once it has run. + + A source produces the diff of its own writes so the terminals that read them can be reselected, + without the follow-up needing to know how that output is located. + """ + + async def capture(self, *, since: Timestamp) -> list[NodeDiff]: ... + @dataclass(frozen=True) class LoadedDefinition[DefinitionT: DefinitionModel]: @@ -21,12 +53,26 @@ class LoadedDefinition[DefinitionT: DefinitionModel]: group_id: str +@dataclass(frozen=True) +class PlannedRegeneration: + """One selector's selected requests, tagged with the workflow and dispatch mode that run them.""" + + workflow: WorkflowDefinition + cascade_role: CascadeRole + requests: Sequence[RegenerationRequest] + output: CascadeSourceOutput | None = None + """How to capture this entry's output when it is a cascade source; None for a terminal.""" + + @dataclass(frozen=True) class SelectiveRegenerationPlan: - """The generator runs and artifact generations a merge follow-up should dispatch.""" + """What a merge follow-up should dispatch, one entry per selector that produced requests.""" + + entries: list[PlannedRegeneration] - generator_runs: list[RequestGeneratorDefinitionRun] - artifact_generates: list[RequestArtifactDefinitionGenerate] + def for_role(self, cascade_role: CascadeRole) -> list[PlannedRegeneration]: + """Return the entries whose selector plays the given cascade role, in selector order.""" + return [entry for entry in self.entries if entry.cascade_role is cascade_role] @dataclass(frozen=True) From 8126a1fee44971df935c2269f623bc5ac4e12ff4 Mon Sep 17 00:00:00 2001 From: Pol Michel Date: Tue, 28 Jul 2026 09:58:09 +0200 Subject: [PATCH 02/16] refactor(regeneration): inject a selector list into the merge orchestrator Replace the two explicitly-typed selector fields with an injected list the orchestrator iterates generically, running each selector over one shared modified-kinds computation and repository-escalation set, and tagging each plan entry with the selector's workflow, cascade role, and output. Consolidating submissions routes each entry's requests back through its owning selector. Adding a definition kind is now one new selector at the wiring layer. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01B7rfYnzxsPEm45r3e2NnLm --- .../merge/selective_regen/orchestrator.py | 175 +++++++++++------- 1 file changed, 112 insertions(+), 63 deletions(-) diff --git a/backend/infrahub/core/merge/selective_regen/orchestrator.py b/backend/infrahub/core/merge/selective_regen/orchestrator.py index c6ecdb0bd1..804daa6f45 100644 --- a/backend/infrahub/core/merge/selective_regen/orchestrator.py +++ b/backend/infrahub/core/merge/selective_regen/orchestrator.py @@ -1,6 +1,6 @@ from __future__ import annotations -from typing import TYPE_CHECKING, Protocol +from typing import TYPE_CHECKING, Any, Protocol from infrahub.proposed_change.branch_diff import get_modified_kinds @@ -9,20 +9,18 @@ from .fallbacks import repositories_forcing_full_regeneration from .gate import DefinitionGate from .impacted import ImpactedSubscriberResolver -from .models import SelectiveRegenerationPlan +from .models import CascadeRole, PlannedRegeneration, SelectiveRegenerationPlan if TYPE_CHECKING: import logging + from collections.abc import Sequence from infrahub_sdk.client import InfrahubClient from infrahub_sdk.diff import NodeDiff - from infrahub.generators.models import ProposedChangeGeneratorDefinition, RequestGeneratorDefinitionRun - from infrahub.git.models import RequestArtifactDefinitionGenerate - from infrahub.message_bus.types import ProposedChangeArtifactDefinition - from .definition_selector.base import DefinitionSelectorBase - from .models import DefinitionModel + from .generator_diff_capturer import GeneratorMutationDiffCapturer + from .models import DefinitionModel, RegenerationRequest class RegenerationSelector(Protocol): @@ -30,84 +28,135 @@ class RegenerationSelector(Protocol): async def build_plan(self, diff_summary: list[NodeDiff], target_branch: str) -> SelectiveRegenerationPlan: ... - async def select_artifacts( + async def reselect_from_cascade_output( self, diff_summary: list[NodeDiff], target_branch: str - ) -> list[RequestArtifactDefinitionGenerate]: ... + ) -> list[PlannedRegeneration]: ... + + def consolidate_submissions(self, entries: Sequence[PlannedRegeneration]) -> list[PlannedRegeneration]: ... class MergeSelectiveRegeneration: - """Select the generator and artifact definitions a merge changed, narrowed to affected members. + """Select the definitions a merge changed, narrowed to affected members, across every selector. - Orchestrates the generator and artifact selectors over a single computation of the diff's - modified kinds, returning the combined plan a merge follow-up should dispatch. + Runs each injected selector over a single computation of the diff's modified kinds and one shared + repository-escalation set, returning one plan entry per selector for the follow-up to dispatch. + Adding a definition kind is one new selector in the injected list, with no change here. """ - def __init__( - self, - generator_selector: DefinitionSelectorBase[ProposedChangeGeneratorDefinition, RequestGeneratorDefinitionRun], - artifact_selector: DefinitionSelectorBase[ProposedChangeArtifactDefinition, RequestArtifactDefinitionGenerate], - ) -> None: - self.generator_selector = generator_selector - self.artifact_selector = artifact_selector + def __init__(self, selectors: Sequence[DefinitionSelectorBase[Any, Any]]) -> None: + self.selectors = selectors async def build_plan(self, diff_summary: list[NodeDiff], target_branch: str) -> SelectiveRegenerationPlan: modified_kinds = get_modified_kinds(diff_summary=diff_summary, branch=target_branch) - generator_definitions = await self.generator_selector.load_definitions(target_branch=target_branch) - artifact_definitions = await self.artifact_selector.load_definitions(target_branch=target_branch) - - # Computed over both kinds so a repository escalated by any missing fingerprint regenerates - # all of its definitions, not only those of the kind that carried the null fingerprint. - definitions: list[DefinitionModel] = [loaded.definition for loaded in generator_definitions] - definitions += [loaded.definition for loaded in artifact_definitions] - forced_repositories = repositories_forcing_full_regeneration(definitions=definitions) - - generator_runs = await self.generator_selector.select( - loaded_definitions=generator_definitions, - forced_repositories=forced_repositories, - diff_summary=diff_summary, - target_branch=target_branch, - modified_kinds=modified_kinds, - ) - artifact_generates = await self.artifact_selector.select( - loaded_definitions=artifact_definitions, - forced_repositories=forced_repositories, - diff_summary=diff_summary, - target_branch=target_branch, - modified_kinds=modified_kinds, - ) - return SelectiveRegenerationPlan(generator_runs=generator_runs, artifact_generates=artifact_generates) - - async def select_artifacts( + loaded_by_selector = [ + (selector, await selector.load_definitions(target_branch=target_branch)) for selector in self.selectors + ] + + # Computed over every selector's definitions so a repository escalated by any missing fingerprint + # regenerates all of its definitions, not only those of the kind that carried the null fingerprint. + all_definitions: list[DefinitionModel] = [ + loaded.definition for _, loaded_definitions in loaded_by_selector for loaded in loaded_definitions + ] + forced_repositories = repositories_forcing_full_regeneration(definitions=all_definitions) + + entries: list[PlannedRegeneration] = [] + for selector, loaded_definitions in loaded_by_selector: + requests = await selector.select( + loaded_definitions=loaded_definitions, + forced_repositories=forced_repositories, + diff_summary=diff_summary, + target_branch=target_branch, + modified_kinds=modified_kinds, + ) + entries.append( + PlannedRegeneration( + workflow=selector.workflow, + cascade_role=selector.cascade_role, + requests=requests, + output=selector.output_capture(requests), + ) + ) + return SelectiveRegenerationPlan(entries=entries) + + async def reselect_from_cascade_output( self, diff_summary: list[NodeDiff], target_branch: str - ) -> list[RequestArtifactDefinitionGenerate]: - """Select only the artifact definitions a diff requires be regenerated, narrowed to members. + ) -> list[PlannedRegeneration]: + """Re-select the definitions a cascade source's own output requires be regenerated. - Runs the artifact half of the selection in isolation, so a diff of an after-merge generator's - own writes reprocesses the artifacts that read them without re-selecting the generators. + Given the diff of what the just-run cascade sources wrote, re-run every non-source selector so + the definitions that read that output are regenerated. The sources are excluded on purpose: + they produced this diff, so re-running them on it would repeat runs already completed. """ modified_kinds = get_modified_kinds(diff_summary=diff_summary, branch=target_branch) - artifact_definitions = await self.artifact_selector.load_definitions(target_branch=target_branch) - forced_repositories = repositories_forcing_full_regeneration( - definitions=[loaded.definition for loaded in artifact_definitions] - ) - return await self.artifact_selector.select( - loaded_definitions=artifact_definitions, - forced_repositories=forced_repositories, - diff_summary=diff_summary, - target_branch=target_branch, - modified_kinds=modified_kinds, - ) + entries: list[PlannedRegeneration] = [] + for selector in self.selectors: + if selector.cascade_role is CascadeRole.SOURCE: + continue + loaded_definitions = await selector.load_definitions(target_branch=target_branch) + forced_repositories = repositories_forcing_full_regeneration( + definitions=[loaded.definition for loaded in loaded_definitions] + ) + requests = await selector.select( + loaded_definitions=loaded_definitions, + forced_repositories=forced_repositories, + diff_summary=diff_summary, + target_branch=target_branch, + modified_kinds=modified_kinds, + ) + entries.append( + PlannedRegeneration( + workflow=selector.workflow, + cascade_role=selector.cascade_role, + requests=requests, + output=selector.output_capture(requests), + ) + ) + return entries + + def consolidate_submissions(self, entries: Sequence[PlannedRegeneration]) -> list[PlannedRegeneration]: + """Combine the given entries' requests through the selector that owns each, one batch per selector. + + Requests are grouped by their selector (matched on workflow), so each selector consolidates its + own kind -- an artifact selected by both the merge diff and a generator's output collapses to a + single request -- without the follow-up knowing how to merge them. + """ + selector_by_workflow = {selector.workflow.name: selector for selector in self.selectors} + requests_by_workflow: dict[str, list[RegenerationRequest]] = {} + for entry in entries: + requests_by_workflow.setdefault(entry.workflow.name, []).extend(entry.requests) + return [ + PlannedRegeneration( + workflow=selector_by_workflow[workflow_name].workflow, + cascade_role=selector_by_workflow[workflow_name].cascade_role, + requests=selector_by_workflow[workflow_name].consolidate(requests), + ) + for workflow_name, requests in requests_by_workflow.items() + ] def build_merge_selective_regeneration( *, client: InfrahubClient, log: logging.Logger | logging.LoggerAdapter[logging.Logger], + output_capturer: GeneratorMutationDiffCapturer, ) -> MergeSelectiveRegeneration: - """Wire a fully-injected selector for one merge follow-up, sharing the gate and impact resolver.""" + """Wire a fully-injected selector for one merge follow-up, sharing the gate and impact resolver. + + The generator selector runs before the artifact selector so the plan awaits generator output + before the artifacts that may read it are selected. The generator selector also holds the output + capturer, so it -- not the follow-up -- owns capturing what its generators wrote. + """ gate = DefinitionGate(log=log) impacted_resolver = ImpactedSubscriberResolver(client=client) return MergeSelectiveRegeneration( - generator_selector=GeneratorSelector(client=client, gate=gate, impacted_resolver=impacted_resolver, log=log), - artifact_selector=ArtifactSelector(client=client, gate=gate, impacted_resolver=impacted_resolver, log=log), + selectors=[ + GeneratorSelector( + client=client, + gate=gate, + impacted_resolver=impacted_resolver, + log=log, + output_capturer=output_capturer, + ), + ArtifactSelector(client=client, gate=gate, impacted_resolver=impacted_resolver, log=log), + ] ) From e811e8c8889474a349c02948b68fa189a5a23b55 Mon Sep 17 00:00:00 2001 From: Pol Michel Date: Tue, 28 Jul 2026 09:58:09 +0200 Subject: [PATCH 03/16] refactor(regeneration): dispatch the plan by cascade role Group the plan's entries by cascade role: sources run to completion, then each source captures its own output and the terminals that read it are reselected from the combined diff, which the owning selector consolidates before submission. The dispatcher no longer narrows requests by kind, captures generator output, or consolidates artifacts -- that logic now lives on the selectors -- so it holds no kind-specific logic. The cascade and its failure isolation are unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01B7rfYnzxsPEm45r3e2NnLm --- backend/infrahub/core/branch/tasks.py | 16 +-- .../core/merge/regeneration_dispatcher.py | 114 ++++++++---------- 2 files changed, 56 insertions(+), 74 deletions(-) diff --git a/backend/infrahub/core/branch/tasks.py b/backend/infrahub/core/branch/tasks.py index ea585caddc..8a50c8e4ad 100644 --- a/backend/infrahub/core/branch/tasks.py +++ b/backend/infrahub/core/branch/tasks.py @@ -494,19 +494,19 @@ async def _build_post_merge_regeneration_dispatcher( component_registry = get_component_registry() diff_coordinator = await component_registry.get_component(DiffCoordinator, db=db, branch=branch) diff_repository = await component_registry.get_component(DiffRepository, db=db, branch=branch) + output_capturer = GeneratorTrackingGroupDiffCapturer( + diff_coordinator=diff_coordinator, + diff_repository=diff_repository, + serializer=DiffSummarySerializer(), + client=get_client(), + branch=branch, + ) return PostMergeRegenerationDispatcher( workflow=get_workflow(), - selector=build_merge_selective_regeneration(client=get_client(), log=log), + selector=build_merge_selective_regeneration(client=get_client(), log=log, output_capturer=output_capturer), summary_cache=DiffSummaryCache( cache=await get_cache(), serializer=DiffSummarySerializer(), key_namespace="branch_merge" ), - generator_diff_capturer=GeneratorTrackingGroupDiffCapturer( - diff_coordinator=diff_coordinator, - diff_repository=diff_repository, - serializer=DiffSummarySerializer(), - client=get_client(), - branch=branch, - ), log=log, ) diff --git a/backend/infrahub/core/merge/regeneration_dispatcher.py b/backend/infrahub/core/merge/regeneration_dispatcher.py index 023b3fd772..a7d55e4710 100644 --- a/backend/infrahub/core/merge/regeneration_dispatcher.py +++ b/backend/infrahub/core/merge/regeneration_dispatcher.py @@ -5,12 +5,11 @@ from typing import TYPE_CHECKING from infrahub import config +from infrahub.core.merge.selective_regen.models import CascadeRole from infrahub.core.timestamp import Timestamp from infrahub.exceptions import ResourceNotFoundError from infrahub.generators.constants import GeneratorDefinitionRunSource from infrahub.workflows.catalogue import ( - REQUEST_ARTIFACT_DEFINITION_GENERATE, - REQUEST_GENERATOR_DEFINITION_RUN, TRIGGER_ARTIFACT_DEFINITION_GENERATE, TRIGGER_GENERATOR_DEFINITION_RUN, ) @@ -18,13 +17,13 @@ if TYPE_CHECKING: from logging import Logger, LoggerAdapter + from infrahub_sdk.diff import NodeDiff + from infrahub.context import InfrahubContext from infrahub.core.diff.summary_cache import DiffSummaryCache - from infrahub.git.models import RequestArtifactDefinitionGenerate from infrahub.services.adapters.workflow import InfrahubWorkflow - from .selective_regen.generator_diff_capturer import GeneratorMutationDiffCapturer - from .selective_regen.models import SelectiveRegenerationPlan + from .selective_regen.models import PlannedRegeneration, SelectiveRegenerationPlan from .selective_regen.orchestrator import RegenerationSelector @@ -49,26 +48,6 @@ async def submit_full_regeneration(*, workflow: InfrahubWorkflow, context: Infra ) -def _consolidate_artifact_generates( - artifact_generates: list[RequestArtifactDefinitionGenerate], -) -> list[RequestArtifactDefinitionGenerate]: - """Merge requests for the same artifact definition into one, unioning their member/limit filters. - - An artifact selected from both the merge diff and a generator's output would otherwise be dispatched - twice; an empty filter means "all members", so it subsumes any specific filter. - """ - consolidated: dict[str, RequestArtifactDefinitionGenerate] = {} - for request in artifact_generates: - merged = consolidated.get(request.artifact_definition_id) - if merged is None: - consolidated[request.artifact_definition_id] = request - continue - members = [] if not merged.members or not request.members else sorted({*merged.members, *request.members}) - limit = [] if not merged.limit or not request.limit else sorted({*merged.limit, *request.limit}) - consolidated[request.artifact_definition_id] = merged.model_copy(update={"members": members, "limit": limit}) - return list(consolidated.values()) - - class PostMergeRegenerationDispatcher: """Decide and submit which generators and artifacts a committed merge should regenerate. @@ -83,13 +62,11 @@ def __init__( workflow: InfrahubWorkflow, selector: RegenerationSelector, summary_cache: DiffSummaryCache, - generator_diff_capturer: GeneratorMutationDiffCapturer, log: Logger | LoggerAdapter[Logger], ) -> None: self.workflow = workflow self.selector = selector self.summary_cache = summary_cache - self.generator_diff_capturer = generator_diff_capturer self.log = log async def dispatch( @@ -141,29 +118,33 @@ async def _dispatch_plan( target_branch: str, plan: SelectiveRegenerationPlan, ) -> None: - generator_cascade = bool(plan.generator_runs) + sources = plan.for_role(CascadeRole.SOURCE) + terminals = plan.for_role(CascadeRole.TERMINAL) + source_runs = [request for entry in sources for request in entry.requests] + generator_cascade = bool(source_runs) cascade_started_at = Timestamp() if generator_cascade else None self.log.debug( - f"Selective post-merge execution: {len(plan.generator_runs)} generator run(s), " - f"{len(plan.artifact_generates)} artifact generation(s)" + f"Selective post-merge execution: {len(source_runs)} cascade-source run(s), " + f"{sum(len(entry.requests) for entry in terminals)} terminal generation(s)" + ("; generator cascade engaged" if generator_cascade else "") ) if cascade_started_at is None: - await self._submit_artifacts(context=context, artifact_generates=plan.artifact_generates) + await self._submit(context=context, entries=terminals) return generator_failed = False - for generator_run in plan.generator_runs: - try: - # Await each generator so its writes have landed before they are captured. - await self.workflow.execute_workflow( - workflow=REQUEST_GENERATOR_DEFINITION_RUN, context=context, parameters={"model": generator_run} - ) - except Exception: - generator_failed = True - self.log.exception("Post-merge generator run failed") + for entry in sources: + for run in entry.requests: + try: + # Await each generator so its writes have landed before they are captured. + await self.workflow.execute_workflow( + workflow=entry.workflow, context=context, parameters={"model": run} + ) + except Exception: + generator_failed = True + self.log.exception("Post-merge generator run failed") if generator_failed: # A failed generator's consuming artifacts cannot be selected from its output, so regenerate @@ -171,50 +152,51 @@ async def _dispatch_plan( await self._submit_full_artifact_regeneration(context=context, target_branch=target_branch) return - targeted = await self._artifacts_from_generator_output( - context=context, - target_branch=target_branch, - since=cascade_started_at, - generator_definition_names=[run.generator_definition.definition_name for run in plan.generator_runs], + targeted = await self._reselect_from_cascade_output( + context=context, target_branch=target_branch, sources=sources, since=cascade_started_at ) if targeted is None: # Every artifact was already regenerated wholesale, which covers the merge-diff selection too. return - # Dispatched only after the capture, so the capture window never sees these artifact generations' - # own writes. - await self._submit_artifacts(context=context, artifact_generates=[*plan.artifact_generates, *targeted]) - - async def _submit_artifacts( - self, *, context: InfrahubContext, artifact_generates: list[RequestArtifactDefinitionGenerate] - ) -> None: - for artifact_generate in _consolidate_artifact_generates(artifact_generates): - await self.workflow.submit_workflow( - workflow=REQUEST_ARTIFACT_DEFINITION_GENERATE, context=context, parameters={"model": artifact_generate} - ) + # Dispatched only after the capture, so the capture window never sees these generations' own writes. + await self._submit(context=context, entries=[*terminals, *targeted]) + + async def _submit(self, *, context: InfrahubContext, entries: list[PlannedRegeneration]) -> None: + """Submit each fire-and-forget request, letting the owning selector consolidate its own kind.""" + for entry in self.selector.consolidate_submissions(entries): + for request in entry.requests: + await self.workflow.submit_workflow( + workflow=entry.workflow, context=context, parameters={"model": request} + ) - async def _artifacts_from_generator_output( + async def _reselect_from_cascade_output( self, *, context: InfrahubContext, target_branch: str, + sources: list[PlannedRegeneration], since: Timestamp, - generator_definition_names: list[str], - ) -> list[RequestArtifactDefinitionGenerate] | None: - """Select the artifacts the just-run generators' writes require be regenerated. + ) -> list[PlannedRegeneration] | None: + """Reselect the fire-and-forget generations the just-run sources' own output requires. - Returns ``None`` after regenerating every artifact wholesale when the generator output cannot be - captured or selected, so a generator's writes can never leave a consuming artifact stale. + Each source captures its own output; the terminals that read it are then reselected from the + combined diff. Returns ``None`` after regenerating every artifact wholesale when that output + cannot be captured or selected, so a source's writes can never leave a consuming artifact stale. """ try: - generator_diff = await self.generator_diff_capturer.capture( - since=since, generator_definition_names=generator_definition_names + captured: list[NodeDiff] = [] + for entry in sources: + if entry.output is not None: + captured.extend(await entry.output.capture(since=since)) + targeted = await self.selector.reselect_from_cascade_output( + diff_summary=captured, target_branch=target_branch ) - targeted = await self.selector.select_artifacts(diff_summary=generator_diff, target_branch=target_branch) except Exception: self.log.exception("Failed to target artifacts from generator output; regenerating all artifacts instead") await self._submit_full_artifact_regeneration(context=context, target_branch=target_branch) return None - self.log.debug(f"Targeted {len(targeted)} artifact definition(s) from generator output") + targeted_count = sum(len(entry.requests) for entry in targeted) + self.log.debug(f"Targeted {targeted_count} artifact definition(s) from generator output") return targeted async def _full_regeneration( From a684c73cf81501da7d327b146231aa336fdd863c Mon Sep 17 00:00:00 2001 From: Pol Michel Date: Tue, 28 Jul 2026 09:58:09 +0200 Subject: [PATCH 04/16] test(regeneration): cover the entries plan, cascade role, output, and consolidation Point the selector test doubles at the workflow/cascade-role metadata and assert on the plan's entries grouped by role. Add unit tests that build_plan tags each entry with its output capture, that for_role filters by role, that the cascade re-selection excludes the sources, that a terminal has no output while a source is not consolidated, that GeneratorSelector.output_capture derives the capture scope from its runs' names, that ArtifactSelector.consolidate merges per definition, that the orchestrator routes consolidation to each owning selector, and that the dispatcher submits what the selector returns. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01B7rfYnzxsPEm45r3e2NnLm --- .../test_merge_selective_regen.py | 5 +- backend/tests/helpers/selective_regen.py | 28 ++- .../definition_selector/conftest.py | 16 +- .../test_artifact_selector.py | 27 +++ .../test_generator_selector.py | 36 +++- .../test_generator_diff_capturer.py | 14 ++ .../selective_regen/test_orchestrator.py | 177 ++++++++++++++-- .../merge/test_regeneration_dispatcher.py | 189 ++++++++++-------- 8 files changed, 388 insertions(+), 104 deletions(-) diff --git a/backend/tests/component/proposed_change/test_merge_selective_regen.py b/backend/tests/component/proposed_change/test_merge_selective_regen.py index 54c8d626e1..6fff4607f7 100644 --- a/backend/tests/component/proposed_change/test_merge_selective_regen.py +++ b/backend/tests/component/proposed_change/test_merge_selective_regen.py @@ -262,11 +262,12 @@ async def test_relevant_kind_change_selects_matching_definitions( ) dispatcher = PostMergeRegenerationDispatcher( workflow=workflow_recorder, - selector=build_merge_selective_regeneration(client=client, log=logging.getLogger("test")), + selector=build_merge_selective_regeneration( + client=client, log=logging.getLogger("test"), output_capturer=generator_output + ), summary_cache=DiffSummaryCache( cache=memory_cache, serializer=DiffSummarySerializer(), key_namespace="branch_merge" ), - generator_diff_capturer=generator_output, log=logging.getLogger("test"), ) await dispatcher.dispatch( diff --git a/backend/tests/helpers/selective_regen.py b/backend/tests/helpers/selective_regen.py index 857797f391..4e2741a29f 100644 --- a/backend/tests/helpers/selective_regen.py +++ b/backend/tests/helpers/selective_regen.py @@ -6,15 +6,35 @@ from infrahub.core.merge.selective_regen.definition_selector.base import DefinitionSelectorBase from infrahub.core.merge.selective_regen.gate import DefinitionGate from infrahub.core.merge.selective_regen.impacted import ImpactedSubscriberResolver -from infrahub.core.merge.selective_regen.models import DefinitionModel, GateResult, LoadedDefinition +from infrahub.core.merge.selective_regen.models import ( + CascadeRole, + DefinitionModel, + GateResult, + LoadedDefinition, +) from infrahub.core.regeneration.models import TargetSelection from infrahub.generators.models import ProposedChangeGeneratorDefinition, RequestGeneratorDefinitionRun from infrahub.git.models import RequestArtifactDefinitionGenerate from infrahub.message_bus.types import ProposedChangeArtifactDefinition +from infrahub.workflows.catalogue import REQUEST_ARTIFACT_DEFINITION_GENERATE, REQUEST_GENERATOR_DEFINITION_RUN if TYPE_CHECKING: from infrahub_sdk.diff import NodeDiff + from infrahub.core.timestamp import Timestamp + + +class RecordingGeneratorDiffCapturer: + """A GeneratorMutationDiffCapturer double: records its calls and returns its diff unchanged.""" + + def __init__(self) -> None: + self.calls: list[tuple[Timestamp, list[str]]] = [] + self.result: list[NodeDiff] = [] + + async def capture(self, *, since: Timestamp, generator_definition_names: list[str]) -> list[NodeDiff]: + self.calls.append((since, generator_definition_names)) + return self.result + class RejectingGate(DefinitionGate): """A gate that selects nothing, so only the untrusted-signal fallback can force a definition.""" @@ -81,6 +101,9 @@ def _should_render(self, *, subscriber_id: str | None, regenerate_all_members: b class GeneratorForcingSelector( ForcingTemplateSelector[ProposedChangeGeneratorDefinition, RequestGeneratorDefinitionRun] ): + workflow = REQUEST_GENERATOR_DEFINITION_RUN + cascade_role = CascadeRole.SOURCE + def _build_request( self, *, definition: ProposedChangeGeneratorDefinition, target_branch: str, members: list[str] ) -> RequestGeneratorDefinitionRun: @@ -92,6 +115,9 @@ def _build_request( class ArtifactForcingSelector( ForcingTemplateSelector[ProposedChangeArtifactDefinition, RequestArtifactDefinitionGenerate] ): + workflow = REQUEST_ARTIFACT_DEFINITION_GENERATE + cascade_role = CascadeRole.TERMINAL + def _build_request( self, *, definition: ProposedChangeArtifactDefinition, target_branch: str, members: list[str] ) -> RequestArtifactDefinitionGenerate: diff --git a/backend/tests/unit/core/merge/selective_regen/definition_selector/conftest.py b/backend/tests/unit/core/merge/selective_regen/definition_selector/conftest.py index 9bfe9a3792..f15b957e1b 100644 --- a/backend/tests/unit/core/merge/selective_regen/definition_selector/conftest.py +++ b/backend/tests/unit/core/merge/selective_regen/definition_selector/conftest.py @@ -9,6 +9,7 @@ from infrahub.core.merge.selective_regen.definition_selector.generator_selector import GeneratorSelector from infrahub.core.merge.selective_regen.gate import DefinitionGate from infrahub.core.merge.selective_regen.impacted import ImpactedSubscriberResolver +from tests.helpers.selective_regen import RecordingGeneratorDiffCapturer @pytest.fixture @@ -38,8 +39,19 @@ def artifact_selector( return ArtifactSelector(client=client, gate=gate, impacted_resolver=impacted_resolver, log=log) +@pytest.fixture +def output_capturer() -> RecordingGeneratorDiffCapturer: + return RecordingGeneratorDiffCapturer() + + @pytest.fixture def generator_selector( - client: InfrahubClient, gate: DefinitionGate, impacted_resolver: ImpactedSubscriberResolver, log: logging.Logger + client: InfrahubClient, + gate: DefinitionGate, + impacted_resolver: ImpactedSubscriberResolver, + log: logging.Logger, + output_capturer: RecordingGeneratorDiffCapturer, ) -> GeneratorSelector: - return GeneratorSelector(client=client, gate=gate, impacted_resolver=impacted_resolver, log=log) + return GeneratorSelector( + client=client, gate=gate, impacted_resolver=impacted_resolver, log=log, output_capturer=output_capturer + ) diff --git a/backend/tests/unit/core/merge/selective_regen/definition_selector/test_artifact_selector.py b/backend/tests/unit/core/merge/selective_regen/definition_selector/test_artifact_selector.py index 12e5eae93a..b54febdc0f 100644 --- a/backend/tests/unit/core/merge/selective_regen/definition_selector/test_artifact_selector.py +++ b/backend/tests/unit/core/merge/selective_regen/definition_selector/test_artifact_selector.py @@ -5,6 +5,7 @@ import pytest +from infrahub.git.models import RequestArtifactDefinitionGenerate from infrahub.message_bus.types import ProposedChangeArtifactDefinition if TYPE_CHECKING: @@ -13,6 +14,15 @@ TARGET_BRANCH = "main" +def _generate(artifact_definition_id: str, members: list[str] | None = None) -> RequestArtifactDefinitionGenerate: + return RequestArtifactDefinitionGenerate( + branch=TARGET_BRANCH, + artifact_definition_id=artifact_definition_id, + artifact_definition_name=artifact_definition_id, + members=members or [], + ) + + def _artifact_definition() -> ProposedChangeArtifactDefinition: return ProposedChangeArtifactDefinition( definition_id="def-1", @@ -40,6 +50,23 @@ def test_build_request_carries_definition_identity_and_members(artifact_selector assert request.members == ["m1", "m2"] +def test_output_capture_is_none_for_a_terminal(artifact_selector: ArtifactSelector) -> None: + """An artifact selector is a cascade terminal: it produces no output for the cascade to re-read.""" + assert artifact_selector.output_capture([]) is None + + +def test_consolidate_merges_requests_for_the_same_definition(artifact_selector: ArtifactSelector) -> None: + """A definition selected more than once collapses to one request; member filters union, empty wins.""" + consolidated = artifact_selector.consolidate( + [_generate("ad1", ["m1"]), _generate("ad2"), _generate("ad1", ["m2"]), _generate("ad2", ["m3"])] + ) + + by_def = {request.artifact_definition_id: request for request in consolidated} + assert len(consolidated) == 2 + assert sorted(by_def["ad1"].members) == ["m1", "m2"] + assert by_def["ad2"].members == [] + + @dataclass class RenderCase: name: str diff --git a/backend/tests/unit/core/merge/selective_regen/definition_selector/test_generator_selector.py b/backend/tests/unit/core/merge/selective_regen/definition_selector/test_generator_selector.py index 85d3e358fc..174d5657fc 100644 --- a/backend/tests/unit/core/merge/selective_regen/definition_selector/test_generator_selector.py +++ b/backend/tests/unit/core/merge/selective_regen/definition_selector/test_generator_selector.py @@ -5,18 +5,20 @@ import pytest +from infrahub.core.timestamp import Timestamp from infrahub.generators.models import ProposedChangeGeneratorDefinition if TYPE_CHECKING: from infrahub.core.merge.selective_regen.definition_selector.generator_selector import GeneratorSelector + from tests.helpers.selective_regen import RecordingGeneratorDiffCapturer TARGET_BRANCH = "main" -def _generator_definition() -> ProposedChangeGeneratorDefinition: +def _generator_definition(name: str = "gen") -> ProposedChangeGeneratorDefinition: return ProposedChangeGeneratorDefinition( definition_id="def-1", - definition_name="gen", + definition_name=name, query_name="q", convert_query_response=False, class_name="C", @@ -44,6 +46,36 @@ def test_build_request_threads_branch_definition_and_members(generator_selector: assert request.target_members == ["m1", "m2"] +async def test_output_capture_captures_the_runs_generators_by_name( + generator_selector: GeneratorSelector, output_capturer: RecordingGeneratorDiffCapturer +) -> None: + """output_capture derives the capture scope from each run's generator name, with no follow-up narrowing.""" + runs = [ + generator_selector._build_request( + definition=_generator_definition(name="gen-a"), target_branch=TARGET_BRANCH, members=[] + ), + generator_selector._build_request( + definition=_generator_definition(name="gen-b"), target_branch=TARGET_BRANCH, members=[] + ), + ] + + output = generator_selector.output_capture(runs) + since = Timestamp() + captured = await output.capture(since=since) + + assert output_capturer.calls == [(since, ["gen-a", "gen-b"])] + assert captured is output_capturer.result + + +def test_consolidate_returns_the_runs_unchanged_for_a_source(generator_selector: GeneratorSelector) -> None: + """A cascade source is not consolidated; its runs pass through the default unchanged.""" + runs = [ + generator_selector._build_request(definition=_generator_definition(), target_branch=TARGET_BRANCH, members=[]) + ] + + assert generator_selector.consolidate(runs) == runs + + @dataclass class RenderCase: name: str diff --git a/backend/tests/unit/core/merge/selective_regen/test_generator_diff_capturer.py b/backend/tests/unit/core/merge/selective_regen/test_generator_diff_capturer.py index 1a1b3b58e5..290573dfe3 100644 --- a/backend/tests/unit/core/merge/selective_regen/test_generator_diff_capturer.py +++ b/backend/tests/unit/core/merge/selective_regen/test_generator_diff_capturer.py @@ -12,8 +12,10 @@ from infrahub.core.merge.selective_regen.generator_diff_capturer import ( CAPTURE_DIFF_NAME_PREFIX, GeneratorTrackingGroupDiffCapturer, + GeneratorTrackingOutput, ) from infrahub.core.timestamp import Timestamp +from tests.helpers.selective_regen import RecordingGeneratorDiffCapturer if TYPE_CHECKING: from infrahub_sdk import InfrahubClient @@ -179,3 +181,15 @@ async def test_capture_marks_the_saved_diff_so_it_can_be_identified_later() -> N assert len(coordinator.names) == 1 assert coordinator.names[0].startswith(CAPTURE_DIFF_NAME_PREFIX) + + +async def test_generator_tracking_output_forwards_its_names_and_since_to_the_capturer() -> None: + """GeneratorTrackingOutput binds the generator names to the capturer and forwards the since and diff.""" + capturer = RecordingGeneratorDiffCapturer() + output = GeneratorTrackingOutput(capturer=capturer, definition_names=["gen-a", "gen-b"]) + since = Timestamp() + + captured = await output.capture(since=since) + + assert capturer.calls == [(since, ["gen-a", "gen-b"])] + assert captured is capturer.result diff --git a/backend/tests/unit/core/merge/selective_regen/test_orchestrator.py b/backend/tests/unit/core/merge/selective_regen/test_orchestrator.py index 68d7d62233..f6830f8ea5 100644 --- a/backend/tests/unit/core/merge/selective_regen/test_orchestrator.py +++ b/backend/tests/unit/core/merge/selective_regen/test_orchestrator.py @@ -3,29 +3,66 @@ from typing import TYPE_CHECKING from infrahub.core.merge.selective_regen.definition_selector.base import DefinitionSelectorBase -from infrahub.core.merge.selective_regen.models import DefinitionModel, LoadedDefinition +from infrahub.core.merge.selective_regen.models import ( + CascadeRole, + DefinitionModel, + LoadedDefinition, + PlannedRegeneration, + SelectiveRegenerationPlan, +) from infrahub.core.merge.selective_regen.orchestrator import MergeSelectiveRegeneration from infrahub.generators.models import ProposedChangeGeneratorDefinition, RequestGeneratorDefinitionRun from infrahub.git.models import RequestArtifactDefinitionGenerate from infrahub.message_bus.types import ProposedChangeArtifactDefinition +from infrahub.workflows.catalogue import REQUEST_ARTIFACT_DEFINITION_GENERATE, REQUEST_GENERATOR_DEFINITION_RUN from tests.helpers.diff_summary import node_diff from tests.helpers.selective_regen import ArtifactForcingSelector, GeneratorForcingSelector if TYPE_CHECKING: + from collections.abc import Sequence + from infrahub_sdk.diff import NodeDiff + from infrahub.core.merge.selective_regen.models import CascadeSourceOutput from infrahub.core.regeneration.models import RegenerationTrigger + from infrahub.core.timestamp import Timestamp + from infrahub.workflows.models import WorkflowDefinition TARGET_BRANCH = "main" REPOSITORY_ID = "repo-1" +class _StubOutput: + """A CascadeSourceOutput sentinel; identity is what the wiring assertions check.""" + + async def capture(self, *, since: Timestamp) -> list[NodeDiff]: + return [] + + class _RecordingSelector[DefinitionT: DefinitionModel, RequestT](DefinitionSelectorBase[DefinitionT, RequestT]): """A selector that returns a canned list and records the arguments select was called with.""" - def __init__(self, result: list[RequestT]) -> None: + def __init__( + self, + result: list[RequestT], + *, + workflow: WorkflowDefinition, + cascade_role: CascadeRole, + output: CascadeSourceOutput | None = None, + ) -> None: self.result = result + self.workflow = workflow + self.cascade_role = cascade_role + self._output = output self.calls: list[tuple[list[NodeDiff], str, list[str]]] = [] + self.consolidate_calls: list[list[RequestT]] = [] + + def output_capture(self, requests: Sequence[RequestT]) -> CascadeSourceOutput | None: + return self._output + + def consolidate(self, requests: Sequence[RequestT]) -> Sequence[RequestT]: + self.consolidate_calls.append(list(requests)) + return requests async def load_definitions(self, *, target_branch: str) -> list[LoadedDefinition[DefinitionT]]: return [] @@ -56,7 +93,26 @@ def _node_diff(*, node_id: str, kind: str, branch: str = TARGET_BRANCH) -> NodeD return node_diff(node_id=node_id, kind=kind, branch=branch) +def _entry(cascade_role: CascadeRole) -> PlannedRegeneration: + workflow = ( + REQUEST_GENERATOR_DEFINITION_RUN if cascade_role is CascadeRole.SOURCE else REQUEST_ARTIFACT_DEFINITION_GENERATE + ) + return PlannedRegeneration(workflow=workflow, cascade_role=cascade_role, requests=[]) + + +def test_for_role_returns_the_entries_playing_that_role_in_order() -> None: + """for_role selects the plan entries whose selector plays the given role, preserving their order.""" + source = _entry(CascadeRole.SOURCE) + first_terminal = _entry(CascadeRole.TERMINAL) + second_terminal = _entry(CascadeRole.TERMINAL) + plan = SelectiveRegenerationPlan(entries=[source, first_terminal, second_terminal]) + + assert plan.for_role(CascadeRole.SOURCE) == [source] + assert plan.for_role(CascadeRole.TERMINAL) == [first_terminal, second_terminal] + + async def test_build_plan_shares_modified_kinds_and_assembles_plan() -> None: + """build_plan computes the modified kinds once and returns one entry per selector, in order.""" diff_summary = [ _node_diff(node_id="n1", kind="TestDevice"), _node_diff(node_id="n2", kind="TestSite"), @@ -66,18 +122,26 @@ async def test_build_plan_shares_modified_kinds_and_assembles_plan() -> None: artifact_request = RequestArtifactDefinitionGenerate( artifact_definition_id="art-1", artifact_definition_name="art", branch=TARGET_BRANCH, members=["m1"] ) - generator_selector = _RecordingSelector[ProposedChangeGeneratorDefinition, RequestGeneratorDefinitionRun](result=[]) + generator_output = _StubOutput() + artifact_output = _StubOutput() + generator_selector = _RecordingSelector[ProposedChangeGeneratorDefinition, RequestGeneratorDefinitionRun]( + result=[], workflow=REQUEST_GENERATOR_DEFINITION_RUN, cascade_role=CascadeRole.SOURCE, output=generator_output + ) artifact_selector = _RecordingSelector[ProposedChangeArtifactDefinition, RequestArtifactDefinitionGenerate]( - result=[artifact_request] + result=[artifact_request], + workflow=REQUEST_ARTIFACT_DEFINITION_GENERATE, + cascade_role=CascadeRole.TERMINAL, + output=artifact_output, ) - plan = await MergeSelectiveRegeneration( - generator_selector=generator_selector, artifact_selector=artifact_selector - ).build_plan(diff_summary=diff_summary, target_branch=TARGET_BRANCH) + plan = await MergeSelectiveRegeneration(selectors=[generator_selector, artifact_selector]).build_plan( + diff_summary=diff_summary, target_branch=TARGET_BRANCH + ) - # Each selector's output lands in its own field of the plan. - assert plan.generator_runs == [] - assert plan.artifact_generates == [artifact_request] + assert [(entry.workflow, entry.cascade_role, entry.requests, entry.output) for entry in plan.entries] == [ + (REQUEST_GENERATOR_DEFINITION_RUN, CascadeRole.SOURCE, [], generator_output), + (REQUEST_ARTIFACT_DEFINITION_GENERATE, CascadeRole.TERMINAL, [artifact_request], artifact_output), + ] # modified_kinds is computed once off the target branch (the other-branch entry is excluded) and # the same diff, branch and kinds reach both selectors. @@ -89,6 +153,70 @@ async def test_build_plan_shares_modified_kinds_and_assembles_plan() -> None: assert set(recorded_kinds) == {"TestDevice", "TestSite"} +async def test_reselect_from_cascade_output_excludes_cascade_sources() -> None: + """The diff of a cascade source's own writes re-runs only the non-source selectors. + + Re-running a source on the diff it produced would repeat a run already completed. + """ + diff_summary = [_node_diff(node_id="n1", kind="TestDevice")] + artifact_request = RequestArtifactDefinitionGenerate( + artifact_definition_id="art-1", artifact_definition_name="art", branch=TARGET_BRANCH, members=["m1"] + ) + generator_selector = _RecordingSelector[ProposedChangeGeneratorDefinition, RequestGeneratorDefinitionRun]( + result=[], workflow=REQUEST_GENERATOR_DEFINITION_RUN, cascade_role=CascadeRole.SOURCE + ) + artifact_selector = _RecordingSelector[ProposedChangeArtifactDefinition, RequestArtifactDefinitionGenerate]( + result=[artifact_request], workflow=REQUEST_ARTIFACT_DEFINITION_GENERATE, cascade_role=CascadeRole.TERMINAL + ) + + entries = await MergeSelectiveRegeneration( + selectors=[generator_selector, artifact_selector] + ).reselect_from_cascade_output(diff_summary=diff_summary, target_branch=TARGET_BRANCH) + + assert generator_selector.calls == [] + assert len(artifact_selector.calls) == 1 + assert [(entry.workflow, entry.cascade_role, entry.requests) for entry in entries] == [ + (REQUEST_ARTIFACT_DEFINITION_GENERATE, CascadeRole.TERMINAL, [artifact_request]), + ] + + +async def test_consolidate_submissions_routes_each_workflow_to_its_selector() -> None: + """Each entry's requests are consolidated by the selector that owns its workflow, then tagged back.""" + generator_selector = _RecordingSelector[ProposedChangeGeneratorDefinition, RequestGeneratorDefinitionRun]( + result=[], workflow=REQUEST_GENERATOR_DEFINITION_RUN, cascade_role=CascadeRole.SOURCE + ) + artifact_selector = _RecordingSelector[ProposedChangeArtifactDefinition, RequestArtifactDefinitionGenerate]( + result=[], workflow=REQUEST_ARTIFACT_DEFINITION_GENERATE, cascade_role=CascadeRole.TERMINAL + ) + generator_run = RequestGeneratorDefinitionRun( + branch=TARGET_BRANCH, generator_definition=_generator(fingerprint="fp") + ) + artifact_request = RequestArtifactDefinitionGenerate( + artifact_definition_id="ad1", artifact_definition_name="art", branch=TARGET_BRANCH + ) + entries = [ + PlannedRegeneration( + workflow=REQUEST_GENERATOR_DEFINITION_RUN, cascade_role=CascadeRole.SOURCE, requests=[generator_run] + ), + PlannedRegeneration( + workflow=REQUEST_ARTIFACT_DEFINITION_GENERATE, + cascade_role=CascadeRole.TERMINAL, + requests=[artifact_request], + ), + ] + + result = MergeSelectiveRegeneration(selectors=[generator_selector, artifact_selector]).consolidate_submissions( + entries + ) + + assert generator_selector.consolidate_calls == [[generator_run]] + assert artifact_selector.consolidate_calls == [[artifact_request]] + assert [(entry.workflow, list(entry.requests)) for entry in result] == [ + (REQUEST_GENERATOR_DEFINITION_RUN, [generator_run]), + (REQUEST_ARTIFACT_DEFINITION_GENERATE, [artifact_request]), + ] + + def _generator(*, fingerprint: str | None) -> ProposedChangeGeneratorDefinition: return ProposedChangeGeneratorDefinition( definition_id="gen-def", @@ -131,9 +259,11 @@ def _artifact(*, fingerprint: str | None) -> ProposedChangeArtifactDefinition: async def test_missing_generator_fingerprint_escalates_a_sibling_artifact_in_the_same_repository() -> None: - # A null-fingerprint generator and a populated-fingerprint artifact share a repository. The forced - # set spans both kinds, so the repository is escalated as a whole: both the generator and the - # artifact regenerate every member even though the gate rejects them and no subscriber is impacted. + """A null-fingerprint definition escalates its whole repository across every kind. + + A null-fingerprint generator and a populated-fingerprint artifact share a repository, so both + regenerate every member even though the gate rejects them and no subscriber is impacted. + """ generator_selector = GeneratorForcingSelector( definitions=[_generator(fingerprint=None)], member_ids=["m1", "m2"], @@ -145,9 +275,20 @@ async def test_missing_generator_fingerprint_escalates_a_sibling_artifact_in_the subscriber_by_member={"m1": "s1", "m2": "s2"}, ) - plan = await MergeSelectiveRegeneration( - generator_selector=generator_selector, artifact_selector=artifact_selector - ).build_plan(diff_summary=[], target_branch=TARGET_BRANCH) + plan = await MergeSelectiveRegeneration(selectors=[generator_selector, artifact_selector]).build_plan( + diff_summary=[], target_branch=TARGET_BRANCH + ) - assert [run.target_members for run in plan.generator_runs] == [[]] - assert [generate.members for generate in plan.artifact_generates] == [[]] + generator_entries = plan.for_role(CascadeRole.SOURCE) + artifact_entries = plan.for_role(CascadeRole.TERMINAL) + generator_runs = [ + run for entry in generator_entries for run in entry.requests if isinstance(run, RequestGeneratorDefinitionRun) + ] + artifact_generates = [ + generate + for entry in artifact_entries + for generate in entry.requests + if isinstance(generate, RequestArtifactDefinitionGenerate) + ] + assert [run.target_members for run in generator_runs] == [[]] + assert [generate.members for generate in artifact_generates] == [[]] diff --git a/backend/tests/unit/core/merge/test_regeneration_dispatcher.py b/backend/tests/unit/core/merge/test_regeneration_dispatcher.py index bb6b2c1a9e..4dc151f5a0 100644 --- a/backend/tests/unit/core/merge/test_regeneration_dispatcher.py +++ b/backend/tests/unit/core/merge/test_regeneration_dispatcher.py @@ -12,7 +12,11 @@ from infrahub.core.diff.summary_cache import DiffSummaryCache from infrahub.core.diff.summary_serializer import DiffSummarySerializer from infrahub.core.merge.regeneration_dispatcher import PostMergeRegenerationDispatcher -from infrahub.core.merge.selective_regen.models import SelectiveRegenerationPlan +from infrahub.core.merge.selective_regen.models import ( + CascadeRole, + PlannedRegeneration, + SelectiveRegenerationPlan, +) from infrahub.generators.constants import GeneratorDefinitionRunSource from infrahub.generators.models import ProposedChangeGeneratorDefinition, RequestGeneratorDefinitionRun from infrahub.git.models import RequestArtifactDefinitionGenerate @@ -26,7 +30,7 @@ from tests.adapters.workflow import WorkflowRecorder if TYPE_CHECKING: - from collections.abc import Iterator + from collections.abc import Iterator, Sequence from infrahub.core.timestamp import Timestamp from infrahub.events.models import EventContext @@ -41,6 +45,36 @@ def _summary_cache(cache: MemoryCache) -> DiffSummaryCache: return DiffSummaryCache(cache=cache, serializer=DiffSummarySerializer(), key_namespace="branch_merge") +def _plan( + *, + generator_runs: list[RequestGeneratorDefinitionRun] | None = None, + artifact_generates: list[RequestArtifactDefinitionGenerate] | None = None, + source_output: _FakeSourceOutput | None = None, +) -> SelectiveRegenerationPlan: + """Build a plan the way the orchestrator does: one entry per selector, tagged by cascade role.""" + return SelectiveRegenerationPlan( + entries=[ + PlannedRegeneration( + workflow=REQUEST_GENERATOR_DEFINITION_RUN, + cascade_role=CascadeRole.SOURCE, + requests=generator_runs or [], + output=source_output, + ), + PlannedRegeneration( + workflow=REQUEST_ARTIFACT_DEFINITION_GENERATE, + cascade_role=CascadeRole.TERMINAL, + requests=artifact_generates or [], + ), + ] + ) + + +def _submitted_entry(requests: list[RequestArtifactDefinitionGenerate]) -> PlannedRegeneration: + return PlannedRegeneration( + workflow=REQUEST_ARTIFACT_DEFINITION_GENERATE, cascade_role=CascadeRole.TERMINAL, requests=requests + ) + + class _FakeSelector: """A RegenerationSelector that returns a canned plan or raises, recording its invocations.""" @@ -50,40 +84,40 @@ def __init__( plan: SelectiveRegenerationPlan | None = None, error: Exception | None = None, artifact_plan: list[RequestArtifactDefinitionGenerate] | None = None, + submissions: list[PlannedRegeneration] | None = None, ) -> None: self._plan = plan self._error = error self._artifact_plan = artifact_plan or [] + self._submissions = submissions self.calls = 0 - self.select_artifacts_diffs: list[list] = [] + self.reselect_diffs: list[list] = [] async def build_plan(self, diff_summary: list, target_branch: str) -> SelectiveRegenerationPlan: self.calls += 1 if self._error is not None: raise self._error - return ( - self._plan - if self._plan is not None - else SelectiveRegenerationPlan(generator_runs=[], artifact_generates=[]) - ) + return self._plan if self._plan is not None else _plan() + + async def reselect_from_cascade_output(self, diff_summary: list, target_branch: str) -> list[PlannedRegeneration]: + self.reselect_diffs.append(diff_summary) + return [_submitted_entry(self._artifact_plan)] - async def select_artifacts(self, diff_summary: list, target_branch: str) -> list[RequestArtifactDefinitionGenerate]: - self.select_artifacts_diffs.append(diff_summary) - return self._artifact_plan + def consolidate_submissions(self, entries: Sequence[PlannedRegeneration]) -> list[PlannedRegeneration]: + """Return the canned submissions when set, otherwise the entries unchanged.""" + return self._submissions if self._submissions is not None else list(entries) -class _FakeCapturer: - """A GeneratorMutationDiffCapturer returning a canned diff summary or raising, recording its calls.""" +class _FakeSourceOutput: + """A CascadeSourceOutput returning a canned diff or raising, recording its capture calls.""" def __init__(self, *, diff_summary: list | None = None, error: Exception | None = None) -> None: self._diff_summary = diff_summary if diff_summary is not None else [] self._error = error self.calls = 0 - self.definition_names: list[list[str]] = [] - async def capture(self, *, since: Timestamp, generator_definition_names: list[str]) -> list: + async def capture(self, *, since: Timestamp) -> list: self.calls += 1 - self.definition_names.append(generator_definition_names) if self._error is not None: raise self._error return self._diff_summary @@ -128,7 +162,7 @@ def _context() -> InfrahubContext: ) -def _plan_with_one_of_each() -> SelectiveRegenerationPlan: +def _plan_with_one_of_each(source_output: _FakeSourceOutput | None = None) -> SelectiveRegenerationPlan: generator_definition = ProposedChangeGeneratorDefinition( definition_id="gd1", definition_name="gen", @@ -145,13 +179,14 @@ def _plan_with_one_of_each() -> SelectiveRegenerationPlan: query_payload="query { TestDevice { edges { node { id } } } }", repository_id="repo-1", ) - return SelectiveRegenerationPlan( + return _plan( generator_runs=[RequestGeneratorDefinitionRun(branch=TARGET_BRANCH, generator_definition=generator_definition)], artifact_generates=[ RequestArtifactDefinitionGenerate( branch=TARGET_BRANCH, artifact_definition_id="ad1", artifact_definition_name="art" ) ], + source_output=source_output, ) @@ -175,20 +210,20 @@ def _generator_run(*, definition_id: str) -> RequestGeneratorDefinitionRun: return RequestGeneratorDefinitionRun(branch=TARGET_BRANCH, generator_definition=generator_definition) -def _plan_with_two_generators() -> SelectiveRegenerationPlan: - return SelectiveRegenerationPlan( +def _plan_with_two_generators(source_output: _FakeSourceOutput | None = None) -> SelectiveRegenerationPlan: + return _plan( generator_runs=[_generator_run(definition_id="gd1"), _generator_run(definition_id="gd2")], artifact_generates=[ RequestArtifactDefinitionGenerate( branch=TARGET_BRANCH, artifact_definition_id="ad1", artifact_definition_name="art" ) ], + source_output=source_output, ) def _plan_with_only_artifacts() -> SelectiveRegenerationPlan: - return SelectiveRegenerationPlan( - generator_runs=[], + return _plan( artifact_generates=[ RequestArtifactDefinitionGenerate( branch=TARGET_BRANCH, artifact_definition_id="ad1", artifact_definition_name="art" @@ -201,13 +236,11 @@ def _dispatcher( selector: _FakeSelector, cache: DiffSummaryCache, recorder: WorkflowRecorder, - capturer: _FakeCapturer | None = None, ) -> PostMergeRegenerationDispatcher: return PostMergeRegenerationDispatcher( workflow=recorder, selector=selector, summary_cache=cache, - generator_diff_capturer=capturer or _FakeCapturer(), log=logging.getLogger("test"), ) @@ -303,7 +336,7 @@ async def test_malformed_summary_submits_full_regeneration() -> None: async def test_empty_plan_dispatches_nothing() -> None: recorder = WorkflowRecorder() - selector = _FakeSelector(plan=SelectiveRegenerationPlan(generator_runs=[], artifact_generates=[])) + selector = _FakeSelector(plan=_plan()) cache = _summary_cache(MemoryCache()) await cache.set(diff_id=DIFF_ID, diff_summary=[]) @@ -337,22 +370,24 @@ def _targeted_artifact() -> RequestArtifactDefinitionGenerate: async def test_merge_targets_artifacts_from_generator_output() -> None: recorder = WorkflowRecorder() - selector = _FakeSelector(plan=_plan_with_one_of_each(), artifact_plan=[_targeted_artifact()]) - capturer = _FakeCapturer(diff_summary=[{"kind": "TestDevice"}]) + source_output = _FakeSourceOutput(diff_summary=[{"kind": "TestDevice"}]) + selector = _FakeSelector( + plan=_plan_with_one_of_each(source_output=source_output), artifact_plan=[_targeted_artifact()] + ) cache = _summary_cache(MemoryCache()) await cache.set(diff_id=DIFF_ID, diff_summary=[]) - await _dispatcher(selector, cache, recorder, capturer).dispatch( + await _dispatcher(selector, cache, recorder).dispatch( context=_context(), target_branch=TARGET_BRANCH, merge_diff_cache_key=DIFF_ID ) - # The generator is awaited (not submitted), its output is captured, and the artifacts are selected - # from that captured diff -- alongside the merge-diff artifact -- with no blanket regeneration. + # The generator is awaited (not submitted), its output is captured by the source itself, and the + # terminals are selected from that captured diff -- alongside the merge-diff artifact -- with no + # blanket regeneration. assert [call["workflow"] for call in recorder.execute_calls] == [REQUEST_GENERATOR_DEFINITION_RUN] assert recorder.get_submit_calls_for(REQUEST_GENERATOR_DEFINITION_RUN) == [] - assert capturer.calls == 1 - assert capturer.definition_names == [["gen"]] - assert selector.select_artifacts_diffs == [[{"kind": "TestDevice"}]] + assert source_output.calls == 1 + assert selector.reselect_diffs == [[{"kind": "TestDevice"}]] submitted = [ call["parameters"]["model"].artifact_definition_name for call in recorder.get_submit_calls_for(REQUEST_ARTIFACT_DEFINITION_GENERATE) @@ -363,18 +398,18 @@ async def test_merge_targets_artifacts_from_generator_output() -> None: async def test_awaits_every_generator_before_capturing_output() -> None: recorder = WorkflowRecorder() - selector = _FakeSelector(plan=_plan_with_two_generators()) - capturer = _FakeCapturer(diff_summary=[{"kind": "TestDevice"}]) + source_output = _FakeSourceOutput(diff_summary=[{"kind": "TestDevice"}]) + selector = _FakeSelector(plan=_plan_with_two_generators(source_output=source_output)) cache = _summary_cache(MemoryCache()) await cache.set(diff_id=DIFF_ID, diff_summary=[]) - await _dispatcher(selector, cache, recorder, capturer).dispatch( + await _dispatcher(selector, cache, recorder).dispatch( context=_context(), target_branch=TARGET_BRANCH, merge_diff_cache_key=DIFF_ID ) - # Racing the tail would capture against a partially-mutated graph. - assert capturer.calls == 1 - assert capturer.definition_names == [["gd1", "gd2"]] + # Both generators are awaited before the single capture; racing the tail would capture against a + # partially-mutated graph. + assert source_output.calls == 1 assert [(call["kind"], call["workflow"]) for call in recorder.calls] == [ ("execute", REQUEST_GENERATOR_DEFINITION_RUN), ("execute", REQUEST_GENERATOR_DEFINITION_RUN), @@ -385,16 +420,15 @@ async def test_awaits_every_generator_before_capturing_output() -> None: async def test_merge_without_generator_keeps_selective_artifacts() -> None: recorder = WorkflowRecorder() selector = _FakeSelector(plan=_plan_with_only_artifacts()) - capturer = _FakeCapturer() cache = _summary_cache(MemoryCache()) await cache.set(diff_id=DIFF_ID, diff_summary=[]) - await _dispatcher(selector, cache, recorder, capturer).dispatch( + await _dispatcher(selector, cache, recorder).dispatch( context=_context(), target_branch=TARGET_BRANCH, merge_diff_cache_key=DIFF_ID ) - # No generator ran, so no output is captured and the artifact selection stays narrow. - assert capturer.calls == 0 + # No source ran, so no output is captured and the artifact selection stays narrow. + assert selector.reselect_diffs == [] assert len(recorder.get_submit_calls_for(REQUEST_ARTIFACT_DEFINITION_GENERATE)) == 1 assert recorder.get_submit_calls_for(TRIGGER_ARTIFACT_DEFINITION_GENERATE) == [] assert recorder.execute_calls == [] @@ -402,12 +436,12 @@ async def test_merge_without_generator_keeps_selective_artifacts() -> None: async def test_generator_output_capture_failure_falls_back_to_blanket_artifacts() -> None: recorder = WorkflowRecorder() - selector = _FakeSelector(plan=_plan_with_one_of_each()) - capturer = _FakeCapturer(error=RuntimeError("capture boom")) + source_output = _FakeSourceOutput(error=RuntimeError("capture boom")) + selector = _FakeSelector(plan=_plan_with_one_of_each(source_output=source_output)) cache = _summary_cache(MemoryCache()) await cache.set(diff_id=DIFF_ID, diff_summary=[]) - await _dispatcher(selector, cache, recorder, capturer).dispatch( + await _dispatcher(selector, cache, recorder).dispatch( context=_context(), target_branch=TARGET_BRANCH, merge_diff_cache_key=DIFF_ID ) @@ -421,12 +455,12 @@ async def test_generator_output_capture_failure_falls_back_to_blanket_artifacts( async def test_generator_run_failure_is_isolated_and_regenerates_artifacts_not_generators() -> None: recorder = _FailingGeneratorRecorder(fail_definition="gd1") - selector = _FakeSelector(plan=_plan_with_two_generators()) - capturer = _FakeCapturer(diff_summary=[{"kind": "TestDevice"}]) + source_output = _FakeSourceOutput(diff_summary=[{"kind": "TestDevice"}]) + selector = _FakeSelector(plan=_plan_with_two_generators(source_output=source_output)) cache = _summary_cache(MemoryCache()) await cache.set(diff_id=DIFF_ID, diff_summary=[]) - await _dispatcher(selector, cache, recorder, capturer).dispatch( + await _dispatcher(selector, cache, recorder).dispatch( context=_context(), target_branch=TARGET_BRANCH, merge_diff_cache_key=DIFF_ID ) @@ -441,43 +475,40 @@ async def test_generator_run_failure_is_isolated_and_regenerates_artifacts_not_g ] == [TARGET_BRANCH] assert recorder.get_submit_calls_for(REQUEST_ARTIFACT_DEFINITION_GENERATE) == [] assert recorder.get_submit_calls_for(TRIGGER_GENERATOR_DEFINITION_RUN) == [] - assert capturer.calls == 0 + # A failed generator short-circuits to blanket regeneration; its output is never captured. + assert source_output.calls == 0 + +async def test_merge_submits_what_the_selector_consolidates() -> None: + """The dispatcher submits exactly the entries the selector's consolidation returns, via their workflow. -async def test_merge_consolidates_artifacts_selected_by_both_diffs() -> None: + Consolidating the requests (deduping a definition selected by more than one diff) is the selector's + job, unit-tested on the selector; here the dispatcher must submit that result verbatim. + """ recorder = WorkflowRecorder() - plan = SelectiveRegenerationPlan( - generator_runs=[_generator_run(definition_id="gd1")], - artifact_generates=[ - RequestArtifactDefinitionGenerate( - branch=TARGET_BRANCH, artifact_definition_id="ad1", artifact_definition_name="art", members=["m1"] - ), - RequestArtifactDefinitionGenerate( - branch=TARGET_BRANCH, artifact_definition_id="ad2", artifact_definition_name="art2" - ), - ], - ) - generator_output = [ - RequestArtifactDefinitionGenerate( - branch=TARGET_BRANCH, artifact_definition_id="ad1", artifact_definition_name="art", members=["m2"] - ), - RequestArtifactDefinitionGenerate( - branch=TARGET_BRANCH, artifact_definition_id="ad2", artifact_definition_name="art2", members=["m3"] - ), + consolidated = [ + _submitted_entry( + [ + RequestArtifactDefinitionGenerate( + branch=TARGET_BRANCH, artifact_definition_id="ad1", artifact_definition_name="art" + ), + RequestArtifactDefinitionGenerate( + branch=TARGET_BRANCH, artifact_definition_id="ad2", artifact_definition_name="art2" + ), + ] + ) ] - selector = _FakeSelector(plan=plan, artifact_plan=generator_output) - capturer = _FakeCapturer(diff_summary=[{"kind": "TestDevice"}]) + source_output = _FakeSourceOutput(diff_summary=[{"kind": "TestDevice"}]) + selector = _FakeSelector(plan=_plan_with_one_of_each(source_output=source_output), submissions=consolidated) cache = _summary_cache(MemoryCache()) await cache.set(diff_id=DIFF_ID, diff_summary=[]) - await _dispatcher(selector, cache, recorder, capturer).dispatch( + await _dispatcher(selector, cache, recorder).dispatch( context=_context(), target_branch=TARGET_BRANCH, merge_diff_cache_key=DIFF_ID ) - submits = recorder.get_submit_calls_for(REQUEST_ARTIFACT_DEFINITION_GENERATE) - by_def = {call["parameters"]["model"].artifact_definition_id: call["parameters"]["model"] for call in submits} - # One request per definition -- ad1, selected by both diffs, is not dispatched twice. - assert len(submits) == 2 - # Member filters are unioned; an unfiltered (all-members) request wins. - assert sorted(by_def["ad1"].members) == ["m1", "m2"] - assert by_def["ad2"].members == [] + submitted = [ + call["parameters"]["model"].artifact_definition_id + for call in recorder.get_submit_calls_for(REQUEST_ARTIFACT_DEFINITION_GENERATE) + ] + assert submitted == ["ad1", "ad2"] From 5fa5bff8cd957df42053e4761789bd9a93c90a9f Mon Sep 17 00:00:00 2001 From: Pol Michel Date: Tue, 28 Jul 2026 10:56:59 +0200 Subject: [PATCH 05/16] fix(regeneration): escalate cascade re-selection across all non-source selectors reselect_from_cascade_output computed the missing-fingerprint escalation per selector, so a repository whose null-fingerprint definition belonged to one terminal kind would not escalate a sibling terminal kind sharing that repository. Aggregate the loaded definitions across every non-source selector before computing the forced set, matching the repository-wide fallback the merge diff selection applies. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01B7rfYnzxsPEm45r3e2NnLm --- .../merge/selective_regen/orchestrator.py | 21 ++++++++++++------- .../selective_regen/test_orchestrator.py | 20 ++++++++++++++++++ 2 files changed, 34 insertions(+), 7 deletions(-) diff --git a/backend/infrahub/core/merge/selective_regen/orchestrator.py b/backend/infrahub/core/merge/selective_regen/orchestrator.py index 804daa6f45..4b1c1e3f52 100644 --- a/backend/infrahub/core/merge/selective_regen/orchestrator.py +++ b/backend/infrahub/core/merge/selective_regen/orchestrator.py @@ -88,14 +88,21 @@ async def reselect_from_cascade_output( they produced this diff, so re-running them on it would repeat runs already completed. """ modified_kinds = get_modified_kinds(diff_summary=diff_summary, branch=target_branch) + loaded_by_selector = [ + (selector, await selector.load_definitions(target_branch=target_branch)) + for selector in self.selectors + if selector.cascade_role is not CascadeRole.SOURCE + ] + + # Aggregated over every non-source selector so a repository escalated by any missing fingerprint + # regenerates all of its definitions, not only the kind that carried the null fingerprint. + all_definitions: list[DefinitionModel] = [ + loaded.definition for _, loaded_definitions in loaded_by_selector for loaded in loaded_definitions + ] + forced_repositories = repositories_forcing_full_regeneration(definitions=all_definitions) + entries: list[PlannedRegeneration] = [] - for selector in self.selectors: - if selector.cascade_role is CascadeRole.SOURCE: - continue - loaded_definitions = await selector.load_definitions(target_branch=target_branch) - forced_repositories = repositories_forcing_full_regeneration( - definitions=[loaded.definition for loaded in loaded_definitions] - ) + for selector, loaded_definitions in loaded_by_selector: requests = await selector.select( loaded_definitions=loaded_definitions, forced_repositories=forced_repositories, diff --git a/backend/tests/unit/core/merge/selective_regen/test_orchestrator.py b/backend/tests/unit/core/merge/selective_regen/test_orchestrator.py index f6830f8ea5..0075d06cb7 100644 --- a/backend/tests/unit/core/merge/selective_regen/test_orchestrator.py +++ b/backend/tests/unit/core/merge/selective_regen/test_orchestrator.py @@ -180,6 +180,26 @@ async def test_reselect_from_cascade_output_excludes_cascade_sources() -> None: ] +async def test_reselect_from_cascade_output_escalates_across_terminals_sharing_a_repository() -> None: + """A null-fingerprint terminal escalates its whole repository, including a sibling terminal. + + The missing-fingerprint set is aggregated over every non-source selector before selection, so it + keeps the repository-wide fallback build_plan applies rather than escalating each kind in isolation. + """ + unpopulated_terminal = ArtifactForcingSelector( + definitions=[_artifact(fingerprint=None)], member_ids=["m1"], subscriber_by_member={"m1": "s1"} + ) + populated_terminal = ArtifactForcingSelector( + definitions=[_artifact(fingerprint="fp")], member_ids=["m2"], subscriber_by_member={"m2": "s2"} + ) + + entries = await MergeSelectiveRegeneration( + selectors=[unpopulated_terminal, populated_terminal] + ).reselect_from_cascade_output(diff_summary=[], target_branch=TARGET_BRANCH) + + assert [len(entry.requests) for entry in entries] == [1, 1] + + async def test_consolidate_submissions_routes_each_workflow_to_its_selector() -> None: """Each entry's requests are consolidated by the selector that owns its workflow, then tagged back.""" generator_selector = _RecordingSelector[ProposedChangeGeneratorDefinition, RequestGeneratorDefinitionRun]( From 9f5bb499f7a00d400ee743349011c83c084ee19e Mon Sep 17 00:00:00 2001 From: Pol Michel Date: Tue, 28 Jul 2026 11:06:32 +0200 Subject: [PATCH 06/16] fix(regeneration): reject a cascade plan where source and output capture disagree build_plan accepted a source selector that captured no output, so a misconfigured source would run while its terminals were silently never reselected. Validate at plan construction that a cascade source carries an output capture and a terminal carries none, raising a wiring error the follow-up turns into safe full regeneration. Fold the shared entry construction into one helper so both the initial plan and the cascade re-selection enforce the invariant. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01B7rfYnzxsPEm45r3e2NnLm --- .../merge/selective_regen/orchestrator.py | 49 ++++++++++----- backend/tests/helpers/selective_regen.py | 13 ++++ .../selective_regen/test_orchestrator.py | 61 +++++++++++++++---- 3 files changed, 94 insertions(+), 29 deletions(-) diff --git a/backend/infrahub/core/merge/selective_regen/orchestrator.py b/backend/infrahub/core/merge/selective_regen/orchestrator.py index 4b1c1e3f52..ca06d4e25b 100644 --- a/backend/infrahub/core/merge/selective_regen/orchestrator.py +++ b/backend/infrahub/core/merge/selective_regen/orchestrator.py @@ -68,14 +68,7 @@ async def build_plan(self, diff_summary: list[NodeDiff], target_branch: str) -> target_branch=target_branch, modified_kinds=modified_kinds, ) - entries.append( - PlannedRegeneration( - workflow=selector.workflow, - cascade_role=selector.cascade_role, - requests=requests, - output=selector.output_capture(requests), - ) - ) + entries.append(self._plan_entry(selector, requests)) return SelectiveRegenerationPlan(entries=entries) async def reselect_from_cascade_output( @@ -110,16 +103,40 @@ async def reselect_from_cascade_output( target_branch=target_branch, modified_kinds=modified_kinds, ) - entries.append( - PlannedRegeneration( - workflow=selector.workflow, - cascade_role=selector.cascade_role, - requests=requests, - output=selector.output_capture(requests), - ) - ) + entries.append(self._plan_entry(selector, requests)) return entries + def _plan_entry( + self, selector: DefinitionSelectorBase[Any, Any], requests: Sequence[RegenerationRequest] + ) -> PlannedRegeneration: + """Build the entry for one selector's requests, enforcing that only a source carries cascade output. + + A source feeds the cascade, so it must capture output for its terminals to reselect from; a + terminal ends the chain, so it must not. A mismatch is a wiring error caught here rather than a + source that runs but whose terminals are silently never reselected. + + Raises: + ValueError: When a source carries no output capture, or a terminal carries one. + + """ + output = selector.output_capture(requests) + if selector.cascade_role is CascadeRole.SOURCE and output is None: + raise ValueError( + f"cascade source {selector.workflow.name!r} produced no output capture; " + "a source must capture the output its terminals reselect from" + ) + if selector.cascade_role is CascadeRole.TERMINAL and output is not None: + raise ValueError( + f"cascade terminal {selector.workflow.name!r} produced an output capture; " + "only a source feeds the cascade" + ) + return PlannedRegeneration( + workflow=selector.workflow, + cascade_role=selector.cascade_role, + requests=requests, + output=output, + ) + def consolidate_submissions(self, entries: Sequence[PlannedRegeneration]) -> list[PlannedRegeneration]: """Combine the given entries' requests through the selector that owns each, one batch per selector. diff --git a/backend/tests/helpers/selective_regen.py b/backend/tests/helpers/selective_regen.py index 4e2741a29f..cf91b377d1 100644 --- a/backend/tests/helpers/selective_regen.py +++ b/backend/tests/helpers/selective_regen.py @@ -8,6 +8,7 @@ from infrahub.core.merge.selective_regen.impacted import ImpactedSubscriberResolver from infrahub.core.merge.selective_regen.models import ( CascadeRole, + CascadeSourceOutput, DefinitionModel, GateResult, LoadedDefinition, @@ -19,11 +20,20 @@ from infrahub.workflows.catalogue import REQUEST_ARTIFACT_DEFINITION_GENERATE, REQUEST_GENERATOR_DEFINITION_RUN if TYPE_CHECKING: + from collections.abc import Sequence + from infrahub_sdk.diff import NodeDiff from infrahub.core.timestamp import Timestamp +class StubCascadeSourceOutput: + """A CascadeSourceOutput that captures nothing, for source doubles that only exercise selection.""" + + async def capture(self, *, since: Timestamp) -> list[NodeDiff]: + return [] + + class RecordingGeneratorDiffCapturer: """A GeneratorMutationDiffCapturer double: records its calls and returns its diff unchanged.""" @@ -104,6 +114,9 @@ class GeneratorForcingSelector( workflow = REQUEST_GENERATOR_DEFINITION_RUN cascade_role = CascadeRole.SOURCE + def output_capture(self, requests: Sequence[RequestGeneratorDefinitionRun]) -> CascadeSourceOutput: + return StubCascadeSourceOutput() + def _build_request( self, *, definition: ProposedChangeGeneratorDefinition, target_branch: str, members: list[str] ) -> RequestGeneratorDefinitionRun: diff --git a/backend/tests/unit/core/merge/selective_regen/test_orchestrator.py b/backend/tests/unit/core/merge/selective_regen/test_orchestrator.py index 0075d06cb7..58b773bd7f 100644 --- a/backend/tests/unit/core/merge/selective_regen/test_orchestrator.py +++ b/backend/tests/unit/core/merge/selective_regen/test_orchestrator.py @@ -2,6 +2,8 @@ from typing import TYPE_CHECKING +import pytest + from infrahub.core.merge.selective_regen.definition_selector.base import DefinitionSelectorBase from infrahub.core.merge.selective_regen.models import ( CascadeRole, @@ -16,7 +18,7 @@ from infrahub.message_bus.types import ProposedChangeArtifactDefinition from infrahub.workflows.catalogue import REQUEST_ARTIFACT_DEFINITION_GENERATE, REQUEST_GENERATOR_DEFINITION_RUN from tests.helpers.diff_summary import node_diff -from tests.helpers.selective_regen import ArtifactForcingSelector, GeneratorForcingSelector +from tests.helpers.selective_regen import ArtifactForcingSelector, GeneratorForcingSelector, StubCascadeSourceOutput if TYPE_CHECKING: from collections.abc import Sequence @@ -25,20 +27,12 @@ from infrahub.core.merge.selective_regen.models import CascadeSourceOutput from infrahub.core.regeneration.models import RegenerationTrigger - from infrahub.core.timestamp import Timestamp from infrahub.workflows.models import WorkflowDefinition TARGET_BRANCH = "main" REPOSITORY_ID = "repo-1" -class _StubOutput: - """A CascadeSourceOutput sentinel; identity is what the wiring assertions check.""" - - async def capture(self, *, since: Timestamp) -> list[NodeDiff]: - return [] - - class _RecordingSelector[DefinitionT: DefinitionModel, RequestT](DefinitionSelectorBase[DefinitionT, RequestT]): """A selector that returns a canned list and records the arguments select was called with.""" @@ -111,6 +105,49 @@ def test_for_role_returns_the_entries_playing_that_role_in_order() -> None: assert plan.for_role(CascadeRole.TERMINAL) == [first_terminal, second_terminal] +async def test_build_plan_rejects_a_cascade_source_without_an_output_capture() -> None: + """A SOURCE that produces no output capture would silently skip the cascade, so the plan is rejected. + + Failing fast at plan construction turns a misconfigured new source into a caught error and safe full + regeneration, rather than a source that runs but whose terminals are never reselected. + """ + source_without_output = _RecordingSelector[ProposedChangeGeneratorDefinition, RequestGeneratorDefinitionRun]( + result=[], workflow=REQUEST_GENERATOR_DEFINITION_RUN, cascade_role=CascadeRole.SOURCE, output=None + ) + + with pytest.raises( + ValueError, + match=( + r"^cascade source 'request-generator-definition-run' produced no output capture; " + r"a source must capture the output its terminals reselect from$" + ), + ): + await MergeSelectiveRegeneration(selectors=[source_without_output]).build_plan( + diff_summary=[], target_branch=TARGET_BRANCH + ) + + +async def test_build_plan_rejects_a_cascade_terminal_carrying_an_output_capture() -> None: + """A TERMINAL that captures output is a wiring error, since only a source feeds the cascade.""" + terminal_with_output = _RecordingSelector[ProposedChangeArtifactDefinition, RequestArtifactDefinitionGenerate]( + result=[], + workflow=REQUEST_ARTIFACT_DEFINITION_GENERATE, + cascade_role=CascadeRole.TERMINAL, + output=StubCascadeSourceOutput(), + ) + + with pytest.raises( + ValueError, + match=( + r"^cascade terminal 'request_artifact_definitions_generate' produced an output capture; " + r"only a source feeds the cascade$" + ), + ): + await MergeSelectiveRegeneration(selectors=[terminal_with_output]).build_plan( + diff_summary=[], target_branch=TARGET_BRANCH + ) + + async def test_build_plan_shares_modified_kinds_and_assembles_plan() -> None: """build_plan computes the modified kinds once and returns one entry per selector, in order.""" diff_summary = [ @@ -122,8 +159,7 @@ async def test_build_plan_shares_modified_kinds_and_assembles_plan() -> None: artifact_request = RequestArtifactDefinitionGenerate( artifact_definition_id="art-1", artifact_definition_name="art", branch=TARGET_BRANCH, members=["m1"] ) - generator_output = _StubOutput() - artifact_output = _StubOutput() + generator_output = StubCascadeSourceOutput() generator_selector = _RecordingSelector[ProposedChangeGeneratorDefinition, RequestGeneratorDefinitionRun]( result=[], workflow=REQUEST_GENERATOR_DEFINITION_RUN, cascade_role=CascadeRole.SOURCE, output=generator_output ) @@ -131,7 +167,6 @@ async def test_build_plan_shares_modified_kinds_and_assembles_plan() -> None: result=[artifact_request], workflow=REQUEST_ARTIFACT_DEFINITION_GENERATE, cascade_role=CascadeRole.TERMINAL, - output=artifact_output, ) plan = await MergeSelectiveRegeneration(selectors=[generator_selector, artifact_selector]).build_plan( @@ -140,7 +175,7 @@ async def test_build_plan_shares_modified_kinds_and_assembles_plan() -> None: assert [(entry.workflow, entry.cascade_role, entry.requests, entry.output) for entry in plan.entries] == [ (REQUEST_GENERATOR_DEFINITION_RUN, CascadeRole.SOURCE, [], generator_output), - (REQUEST_ARTIFACT_DEFINITION_GENERATE, CascadeRole.TERMINAL, [artifact_request], artifact_output), + (REQUEST_ARTIFACT_DEFINITION_GENERATE, CascadeRole.TERMINAL, [artifact_request], None), ] # modified_kinds is computed once off the target branch (the other-branch entry is excluded) and From 60fe327fa3e7761ceb8139c8981e941af57fe344 Mon Sep 17 00:00:00 2001 From: Pol Michel Date: Wed, 29 Jul 2026 11:53:11 +0200 Subject: [PATCH 07/16] refactor(regeneration): own cascade output in a participant, not the selector Selection and cascade output-capture are separate concerns that shared the generator selector: it held the diff capturer only to forward it and built the cascade output itself. Split them so a selector owns only its kind's selection. A CascadeParticipant pairs a selector with its place in the cascade: a CascadeSource carries a CascadeOutput that builds the capture from the selected requests, a CascadeTerminal carries none. The source/terminal invariant the plan builder enforced at runtime is now structural -- a source cannot be constructed without an output, a terminal has no output to set -- so the validating branch is gone. GeneratorCascadeOutput owns the generator-specific name extraction, keeping the capturer free of that request shape. Rename generator_diff_capturer to generator_output, which now also holds the cascade output builder. Co-Authored-By: Claude Opus 4.8 (1M context) --- backend/infrahub/core/branch/tasks.py | 2 +- .../definition_selector/artifact_selector.py | 3 +- .../definition_selector/base.py | 12 +- .../definition_selector/generator_selector.py | 33 +--- ...r_diff_capturer.py => generator_output.py} | 22 +++ .../core/merge/selective_regen/models.py | 10 ++ .../merge/selective_regen/orchestrator.py | 153 +++++++----------- .../core/merge/selective_regen/participant.py | 60 +++++++ 8 files changed, 150 insertions(+), 145 deletions(-) rename backend/infrahub/core/merge/selective_regen/{generator_diff_capturer.py => generator_output.py} (87%) create mode 100644 backend/infrahub/core/merge/selective_regen/participant.py diff --git a/backend/infrahub/core/branch/tasks.py b/backend/infrahub/core/branch/tasks.py index 8a50c8e4ad..3056de4c83 100644 --- a/backend/infrahub/core/branch/tasks.py +++ b/backend/infrahub/core/branch/tasks.py @@ -33,7 +33,7 @@ ) from infrahub.core.merge.regeneration_dispatcher import PostMergeRegenerationDispatcher, submit_full_regeneration from infrahub.core.merge.schema_analyzer import MergeSchemaAnalyzer -from infrahub.core.merge.selective_regen.generator_diff_capturer import GeneratorTrackingGroupDiffCapturer +from infrahub.core.merge.selective_regen.generator_output import GeneratorTrackingGroupDiffCapturer from infrahub.core.merge.selective_regen.orchestrator import build_merge_selective_regeneration from infrahub.core.merge.write_blocker import MergeWriteBlocker from infrahub.core.migrations.exceptions import MigrationFailureError diff --git a/backend/infrahub/core/merge/selective_regen/definition_selector/artifact_selector.py b/backend/infrahub/core/merge/selective_regen/definition_selector/artifact_selector.py index d96fb63746..6c3b4cfcaa 100644 --- a/backend/infrahub/core/merge/selective_regen/definition_selector/artifact_selector.py +++ b/backend/infrahub/core/merge/selective_regen/definition_selector/artifact_selector.py @@ -12,7 +12,7 @@ from infrahub.message_bus.types import ProposedChangeArtifactDefinition from infrahub.workflows.catalogue import REQUEST_ARTIFACT_DEFINITION_GENERATE -from ..models import CascadeRole, LoadedDefinition +from ..models import LoadedDefinition from .base import DefinitionSelectorBase if TYPE_CHECKING: @@ -24,7 +24,6 @@ class ArtifactSelector(DefinitionSelectorBase[ProposedChangeArtifactDefinition, subscriber_kind = InfrahubKind.ARTIFACT workflow = REQUEST_ARTIFACT_DEFINITION_GENERATE - cascade_role = CascadeRole.TERMINAL def consolidate( self, requests: Sequence[RequestArtifactDefinitionGenerate] diff --git a/backend/infrahub/core/merge/selective_regen/definition_selector/base.py b/backend/infrahub/core/merge/selective_regen/definition_selector/base.py index d06d5d4d00..8764f6122e 100644 --- a/backend/infrahub/core/merge/selective_regen/definition_selector/base.py +++ b/backend/infrahub/core/merge/selective_regen/definition_selector/base.py @@ -20,7 +20,7 @@ from ..gate import DefinitionGate from ..impacted import ImpactedSubscriberResolver - from ..models import CascadeRole, CascadeSourceOutput, LoadedDefinition + from ..models import LoadedDefinition def _narrow_members_filter(rendered_members: list[str], total_members: int) -> list[str]: @@ -46,8 +46,6 @@ class DefinitionSelectorBase[DefinitionT: DefinitionModel, RequestT](ABC): subscriber_kind: str workflow: WorkflowDefinition """The workflow that runs this selector's requests.""" - cascade_role: CascadeRole - """This selector's role in the merge regeneration cascade, which orders how it runs.""" def __init__( self, @@ -138,14 +136,6 @@ async def select( requests.append(self._build_request(definition=definition, target_branch=target_branch, members=members)) return requests - def output_capture(self, requests: Sequence[RequestT]) -> CascadeSourceOutput | None: # noqa: ARG002 - """Return how to capture this selector's output for the cascade, or None if it is not a source. - - Overridden by cascade-source selectors, which know their concrete request type and so can build - the capture without the follow-up narrowing it. - """ - return None - def consolidate(self, requests: Sequence[RequestT]) -> Sequence[RequestT]: """Combine this selector's requests before dispatch; by default each is dispatched as-is. diff --git a/backend/infrahub/core/merge/selective_regen/definition_selector/generator_selector.py b/backend/infrahub/core/merge/selective_regen/definition_selector/generator_selector.py index 19ce2e92c5..0a4dac448f 100644 --- a/backend/infrahub/core/merge/selective_regen/definition_selector/generator_selector.py +++ b/backend/infrahub/core/merge/selective_regen/definition_selector/generator_selector.py @@ -1,7 +1,5 @@ from __future__ import annotations -from typing import TYPE_CHECKING - from infrahub_sdk.protocols import CoreGeneratorDefinition from infrahub.core.constants import InfrahubKind @@ -14,44 +12,15 @@ from infrahub.git.utils import fetch_proposed_change_generator_definition_targets from infrahub.workflows.catalogue import REQUEST_GENERATOR_DEFINITION_RUN -from ..generator_diff_capturer import GeneratorTrackingOutput -from ..models import CascadeRole, CascadeSourceOutput, LoadedDefinition +from ..models import LoadedDefinition from .base import DefinitionSelectorBase -if TYPE_CHECKING: - import logging - from collections.abc import Sequence - - from infrahub_sdk.client import InfrahubClient - - from ..gate import DefinitionGate - from ..generator_diff_capturer import GeneratorMutationDiffCapturer - from ..impacted import ImpactedSubscriberResolver - class GeneratorSelector(DefinitionSelectorBase[ProposedChangeGeneratorDefinition, RequestGeneratorDefinitionRun]): """Selects the generator definitions flagged to execute after a merge, narrowed to affected members.""" subscriber_kind = InfrahubKind.GENERATORINSTANCE workflow = REQUEST_GENERATOR_DEFINITION_RUN - cascade_role = CascadeRole.SOURCE - - def __init__( - self, - client: InfrahubClient, - gate: DefinitionGate, - impacted_resolver: ImpactedSubscriberResolver, - log: logging.Logger | logging.LoggerAdapter[logging.Logger], - output_capturer: GeneratorMutationDiffCapturer, - ) -> None: - super().__init__(client=client, gate=gate, impacted_resolver=impacted_resolver, log=log) - self._output_capturer = output_capturer - - def output_capture(self, requests: Sequence[RequestGeneratorDefinitionRun]) -> CascadeSourceOutput: - return GeneratorTrackingOutput( - capturer=self._output_capturer, - definition_names=[run.generator_definition.definition_name for run in requests], - ) async def load_definitions( self, *, target_branch: str diff --git a/backend/infrahub/core/merge/selective_regen/generator_diff_capturer.py b/backend/infrahub/core/merge/selective_regen/generator_output.py similarity index 87% rename from backend/infrahub/core/merge/selective_regen/generator_diff_capturer.py rename to backend/infrahub/core/merge/selective_regen/generator_output.py index a3c136d813..1e79f11dc7 100644 --- a/backend/infrahub/core/merge/selective_regen/generator_diff_capturer.py +++ b/backend/infrahub/core/merge/selective_regen/generator_output.py @@ -10,6 +10,8 @@ from infrahub.core.timestamp import Timestamp if TYPE_CHECKING: + from collections.abc import Sequence + from infrahub_sdk import InfrahubClient from infrahub_sdk.diff import NodeDiff @@ -17,6 +19,9 @@ from infrahub.core.diff.coordinator import DiffCoordinator from infrahub.core.diff.repository.repository import DiffRepository from infrahub.core.diff.summary_serializer import DiffSummarySerializer + from infrahub.generators.models import RequestGeneratorDefinitionRun + + from .models import CascadeSourceOutput # A generator tracks the nodes it writes into a per-member group named "-". _TRACKING_HASH = re.compile(r"[0-9a-f]{32}$") @@ -52,6 +57,23 @@ async def capture(self, *, since: Timestamp) -> list[NodeDiff]: return await self._capturer.capture(since=since, generator_definition_names=self._definition_names) +class GeneratorCascadeOutput: + """Produces a set of generators' cascade output from the runs selected for them. + + Owns the generator-specific step of reading the definition names off the runs, so the capturer it + holds stays free of that kind's request shape. + """ + + def __init__(self, capturer: GeneratorMutationDiffCapturer) -> None: + self._capturer = capturer + + def for_requests(self, requests: Sequence[RequestGeneratorDefinitionRun]) -> CascadeSourceOutput: + return GeneratorTrackingOutput( + capturer=self._capturer, + definition_names=[run.generator_definition.definition_name for run in requests], + ) + + class GeneratorTrackingGroupDiffCapturer: """Capture a post-merge generator's own writes, scoped to the nodes it tracked. diff --git a/backend/infrahub/core/merge/selective_regen/models.py b/backend/infrahub/core/merge/selective_regen/models.py index 2723027a3b..50be4bb9f8 100644 --- a/backend/infrahub/core/merge/selective_regen/models.py +++ b/backend/infrahub/core/merge/selective_regen/models.py @@ -45,6 +45,16 @@ class CascadeSourceOutput(Protocol): async def capture(self, *, since: Timestamp) -> list[NodeDiff]: ... +class CascadeOutput[RequestT](Protocol): + """Produces the output capture for a cascade source from the requests it selected. + + Bound to a source at wiring time and given that source's selected requests, so the capture is + scoped to what those requests will write without the source itself owning how that is located. + """ + + def for_requests(self, requests: Sequence[RequestT]) -> CascadeSourceOutput: ... + + @dataclass(frozen=True) class LoadedDefinition[DefinitionT: DefinitionModel]: """A candidate definition paired with the id of the group it targets.""" diff --git a/backend/infrahub/core/merge/selective_regen/orchestrator.py b/backend/infrahub/core/merge/selective_regen/orchestrator.py index ca06d4e25b..a882bf6b5e 100644 --- a/backend/infrahub/core/merge/selective_regen/orchestrator.py +++ b/backend/infrahub/core/merge/selective_regen/orchestrator.py @@ -1,6 +1,6 @@ from __future__ import annotations -from typing import TYPE_CHECKING, Any, Protocol +from typing import TYPE_CHECKING, Protocol from infrahub.proposed_change.branch_diff import get_modified_kinds @@ -8,8 +8,10 @@ from .definition_selector.generator_selector import GeneratorSelector from .fallbacks import repositories_forcing_full_regeneration from .gate import DefinitionGate +from .generator_output import GeneratorCascadeOutput from .impacted import ImpactedSubscriberResolver from .models import CascadeRole, PlannedRegeneration, SelectiveRegenerationPlan +from .participant import CascadeSource, CascadeTerminal if TYPE_CHECKING: import logging @@ -18,9 +20,9 @@ from infrahub_sdk.client import InfrahubClient from infrahub_sdk.diff import NodeDiff - from .definition_selector.base import DefinitionSelectorBase - from .generator_diff_capturer import GeneratorMutationDiffCapturer + from .generator_output import GeneratorMutationDiffCapturer from .models import DefinitionModel, RegenerationRequest + from .participant import CascadeParticipant class RegenerationSelector(Protocol): @@ -36,39 +38,18 @@ def consolidate_submissions(self, entries: Sequence[PlannedRegeneration]) -> lis class MergeSelectiveRegeneration: - """Select the definitions a merge changed, narrowed to affected members, across every selector. + """Select the definitions a merge changed, narrowed to affected members, across every participant. - Runs each injected selector over a single computation of the diff's modified kinds and one shared - repository-escalation set, returning one plan entry per selector for the follow-up to dispatch. - Adding a definition kind is one new selector in the injected list, with no change here. + Runs each injected participant's selector over a single computation of the diff's modified kinds and + one shared repository-escalation set, returning one plan entry per participant for the follow-up to + dispatch. Adding a definition kind is one new participant in the injected list, with no change here. """ - def __init__(self, selectors: Sequence[DefinitionSelectorBase[Any, Any]]) -> None: - self.selectors = selectors + def __init__(self, participants: Sequence[CascadeParticipant]) -> None: + self.participants = participants async def build_plan(self, diff_summary: list[NodeDiff], target_branch: str) -> SelectiveRegenerationPlan: - modified_kinds = get_modified_kinds(diff_summary=diff_summary, branch=target_branch) - loaded_by_selector = [ - (selector, await selector.load_definitions(target_branch=target_branch)) for selector in self.selectors - ] - - # Computed over every selector's definitions so a repository escalated by any missing fingerprint - # regenerates all of its definitions, not only those of the kind that carried the null fingerprint. - all_definitions: list[DefinitionModel] = [ - loaded.definition for _, loaded_definitions in loaded_by_selector for loaded in loaded_definitions - ] - forced_repositories = repositories_forcing_full_regeneration(definitions=all_definitions) - - entries: list[PlannedRegeneration] = [] - for selector, loaded_definitions in loaded_by_selector: - requests = await selector.select( - loaded_definitions=loaded_definitions, - forced_repositories=forced_repositories, - diff_summary=diff_summary, - target_branch=target_branch, - modified_kinds=modified_kinds, - ) - entries.append(self._plan_entry(selector, requests)) + entries = await self._plan(self.participants, diff_summary=diff_summary, target_branch=target_branch) return SelectiveRegenerationPlan(entries=entries) async def reselect_from_cascade_output( @@ -76,86 +57,63 @@ async def reselect_from_cascade_output( ) -> list[PlannedRegeneration]: """Re-select the definitions a cascade source's own output requires be regenerated. - Given the diff of what the just-run cascade sources wrote, re-run every non-source selector so - the definitions that read that output are regenerated. The sources are excluded on purpose: - they produced this diff, so re-running them on it would repeat runs already completed. + Given the diff of what the just-run cascade sources wrote, re-run every non-source participant so + the definitions that read that output are regenerated. The sources are excluded on purpose: they + produced this diff, so re-running them on it would repeat runs already completed. """ + participants = [participant for participant in self.participants if participant.role is not CascadeRole.SOURCE] + return await self._plan(participants, diff_summary=diff_summary, target_branch=target_branch) + + async def _plan( + self, participants: Sequence[CascadeParticipant], *, diff_summary: list[NodeDiff], target_branch: str + ) -> list[PlannedRegeneration]: modified_kinds = get_modified_kinds(diff_summary=diff_summary, branch=target_branch) - loaded_by_selector = [ - (selector, await selector.load_definitions(target_branch=target_branch)) - for selector in self.selectors - if selector.cascade_role is not CascadeRole.SOURCE + loaded_by_participant = [ + (participant, await participant.selector.load_definitions(target_branch=target_branch)) + for participant in participants ] - # Aggregated over every non-source selector so a repository escalated by any missing fingerprint - # regenerates all of its definitions, not only the kind that carried the null fingerprint. + # Aggregated over every participant's definitions so a repository escalated by any missing + # fingerprint regenerates all of its definitions, not only the kind that carried the null one. all_definitions: list[DefinitionModel] = [ - loaded.definition for _, loaded_definitions in loaded_by_selector for loaded in loaded_definitions + loaded.definition for _, loaded_definitions in loaded_by_participant for loaded in loaded_definitions ] forced_repositories = repositories_forcing_full_regeneration(definitions=all_definitions) entries: list[PlannedRegeneration] = [] - for selector, loaded_definitions in loaded_by_selector: - requests = await selector.select( + for participant, loaded_definitions in loaded_by_participant: + requests = await participant.selector.select( loaded_definitions=loaded_definitions, forced_repositories=forced_repositories, diff_summary=diff_summary, target_branch=target_branch, modified_kinds=modified_kinds, ) - entries.append(self._plan_entry(selector, requests)) + entries.append(participant.to_entry(requests)) return entries - def _plan_entry( - self, selector: DefinitionSelectorBase[Any, Any], requests: Sequence[RegenerationRequest] - ) -> PlannedRegeneration: - """Build the entry for one selector's requests, enforcing that only a source carries cascade output. - - A source feeds the cascade, so it must capture output for its terminals to reselect from; a - terminal ends the chain, so it must not. A mismatch is a wiring error caught here rather than a - source that runs but whose terminals are silently never reselected. - - Raises: - ValueError: When a source carries no output capture, or a terminal carries one. - - """ - output = selector.output_capture(requests) - if selector.cascade_role is CascadeRole.SOURCE and output is None: - raise ValueError( - f"cascade source {selector.workflow.name!r} produced no output capture; " - "a source must capture the output its terminals reselect from" - ) - if selector.cascade_role is CascadeRole.TERMINAL and output is not None: - raise ValueError( - f"cascade terminal {selector.workflow.name!r} produced an output capture; " - "only a source feeds the cascade" - ) - return PlannedRegeneration( - workflow=selector.workflow, - cascade_role=selector.cascade_role, - requests=requests, - output=output, - ) - def consolidate_submissions(self, entries: Sequence[PlannedRegeneration]) -> list[PlannedRegeneration]: - """Combine the given entries' requests through the selector that owns each, one batch per selector. + """Combine the given entries' requests through the participant that owns each, one batch per workflow. - Requests are grouped by their selector (matched on workflow), so each selector consolidates its - own kind -- an artifact selected by both the merge diff and a generator's output collapses to a - single request -- without the follow-up knowing how to merge them. + Requests are grouped by workflow, so each participant's selector consolidates its own kind -- an + artifact selected by both the merge diff and a generator's output collapses to a single request -- + without the follow-up knowing how to merge them. """ - selector_by_workflow = {selector.workflow.name: selector for selector in self.selectors} + participant_by_workflow = {participant.selector.workflow.name: participant for participant in self.participants} requests_by_workflow: dict[str, list[RegenerationRequest]] = {} for entry in entries: requests_by_workflow.setdefault(entry.workflow.name, []).extend(entry.requests) - return [ - PlannedRegeneration( - workflow=selector_by_workflow[workflow_name].workflow, - cascade_role=selector_by_workflow[workflow_name].cascade_role, - requests=selector_by_workflow[workflow_name].consolidate(requests), + submissions: list[PlannedRegeneration] = [] + for workflow_name, requests in requests_by_workflow.items(): + participant = participant_by_workflow[workflow_name] + submissions.append( + PlannedRegeneration( + workflow=participant.selector.workflow, + cascade_role=participant.role, + requests=participant.selector.consolidate(requests), + ) ) - for workflow_name, requests in requests_by_workflow.items() - ] + return submissions def build_merge_selective_regeneration( @@ -164,23 +122,20 @@ def build_merge_selective_regeneration( log: logging.Logger | logging.LoggerAdapter[logging.Logger], output_capturer: GeneratorMutationDiffCapturer, ) -> MergeSelectiveRegeneration: - """Wire a fully-injected selector for one merge follow-up, sharing the gate and impact resolver. + """Wire the participants for one merge follow-up, sharing the gate and impact resolver. - The generator selector runs before the artifact selector so the plan awaits generator output - before the artifacts that may read it are selected. The generator selector also holds the output - capturer, so it -- not the follow-up -- owns capturing what its generators wrote. + The generator participant runs before the artifact participant so the plan awaits generator output + before the artifacts that may read it are selected. The generator participant is the cascade source + and carries the output capture built from the capturer, so the follow-up need not own that. """ gate = DefinitionGate(log=log) impacted_resolver = ImpactedSubscriberResolver(client=client) return MergeSelectiveRegeneration( - selectors=[ - GeneratorSelector( - client=client, - gate=gate, - impacted_resolver=impacted_resolver, - log=log, - output_capturer=output_capturer, + participants=[ + CascadeSource( + GeneratorSelector(client=client, gate=gate, impacted_resolver=impacted_resolver, log=log), + output=GeneratorCascadeOutput(capturer=output_capturer), ), - ArtifactSelector(client=client, gate=gate, impacted_resolver=impacted_resolver, log=log), + CascadeTerminal(ArtifactSelector(client=client, gate=gate, impacted_resolver=impacted_resolver, log=log)), ] ) diff --git a/backend/infrahub/core/merge/selective_regen/participant.py b/backend/infrahub/core/merge/selective_regen/participant.py new file mode 100644 index 0000000000..e1a6a4f9ca --- /dev/null +++ b/backend/infrahub/core/merge/selective_regen/participant.py @@ -0,0 +1,60 @@ +from __future__ import annotations + +from abc import ABC, abstractmethod +from typing import TYPE_CHECKING, Any + +from .models import CascadeRole, PlannedRegeneration + +if TYPE_CHECKING: + from collections.abc import Sequence + + from .definition_selector.base import DefinitionSelectorBase + from .models import CascadeOutput, CascadeSourceOutput, RegenerationRequest + + +class CascadeParticipant(ABC): + """A selector paired with its place in the merge regeneration cascade. + + Selection is the selector's concern; capturing output for the cascade is a separate one. A + participant binds the two so a source and a terminal differ by composition rather than a flag. + """ + + role: CascadeRole + + def __init__(self, selector: DefinitionSelectorBase[Any, Any]) -> None: + self.selector = selector + + def to_entry(self, requests: Sequence[RegenerationRequest]) -> PlannedRegeneration: + """Turn this participant's selected requests into a plan entry carrying its cascade output.""" + return PlannedRegeneration( + workflow=self.selector.workflow, + cascade_role=self.role, + requests=requests, + output=self._capture(requests), + ) + + @abstractmethod + def _capture(self, requests: Sequence[RegenerationRequest]) -> CascadeSourceOutput | None: + """The output capture for these requests, or None when nothing downstream reselects from it.""" + + +class CascadeSource(CascadeParticipant): + """A participant whose output the cascade re-reads to reselect the terminals that consume it.""" + + role = CascadeRole.SOURCE + + def __init__(self, selector: DefinitionSelectorBase[Any, Any], *, output: CascadeOutput[Any]) -> None: + super().__init__(selector) + self._output = output + + def _capture(self, requests: Sequence[RegenerationRequest]) -> CascadeSourceOutput: + return self._output.for_requests(requests) + + +class CascadeTerminal(CascadeParticipant): + """A participant that ends the cascade -- nothing downstream re-reads what it produces.""" + + role = CascadeRole.TERMINAL + + def _capture(self, requests: Sequence[RegenerationRequest]) -> None: # noqa: ARG002 + return None From f573597e9f18947f53e9743356fce6bf11643be6 Mon Sep 17 00:00:00 2001 From: Pol Michel Date: Wed, 29 Jul 2026 11:53:24 +0200 Subject: [PATCH 08/16] test(regeneration): cover the cascade participant refactor Adapt the selective-regen tests to the participant split: wrap selectors in CascadeSource/CascadeTerminal, add a StubCascadeOutput double, and move the output-capture behaviour test onto GeneratorCascadeOutput. Add test_participant for the source/terminal entry mapping, replacing the two plan-builder tests that asserted the now-unrepresentable source-without-output and terminal-with-output errors. Co-Authored-By: Claude Opus 4.8 (1M context) --- backend/tests/helpers/selective_regen.py | 18 ++- .../definition_selector/conftest.py | 11 +- .../test_artifact_selector.py | 5 - .../test_generator_selector.py | 23 ---- ...f_capturer.py => test_generator_output.py} | 38 +++++- .../selective_regen/test_orchestrator.py | 122 ++++++------------ .../merge/selective_regen/test_participant.py | 55 ++++++++ 7 files changed, 144 insertions(+), 128 deletions(-) rename backend/tests/unit/core/merge/selective_regen/{test_generator_diff_capturer.py => test_generator_output.py} (84%) create mode 100644 backend/tests/unit/core/merge/selective_regen/test_participant.py diff --git a/backend/tests/helpers/selective_regen.py b/backend/tests/helpers/selective_regen.py index cf91b377d1..9fef339afd 100644 --- a/backend/tests/helpers/selective_regen.py +++ b/backend/tests/helpers/selective_regen.py @@ -7,8 +7,6 @@ from infrahub.core.merge.selective_regen.gate import DefinitionGate from infrahub.core.merge.selective_regen.impacted import ImpactedSubscriberResolver from infrahub.core.merge.selective_regen.models import ( - CascadeRole, - CascadeSourceOutput, DefinitionModel, GateResult, LoadedDefinition, @@ -24,6 +22,7 @@ from infrahub_sdk.diff import NodeDiff + from infrahub.core.merge.selective_regen.models import CascadeSourceOutput from infrahub.core.timestamp import Timestamp @@ -34,6 +33,16 @@ async def capture(self, *, since: Timestamp) -> list[NodeDiff]: return [] +class StubCascadeOutput: + """A CascadeOutput yielding a fixed capture, for source doubles that only exercise selection.""" + + def __init__(self, result: CascadeSourceOutput | None = None) -> None: + self._result = result if result is not None else StubCascadeSourceOutput() + + def for_requests(self, requests: Sequence[object]) -> CascadeSourceOutput: + return self._result + + class RecordingGeneratorDiffCapturer: """A GeneratorMutationDiffCapturer double: records its calls and returns its diff unchanged.""" @@ -112,10 +121,6 @@ class GeneratorForcingSelector( ForcingTemplateSelector[ProposedChangeGeneratorDefinition, RequestGeneratorDefinitionRun] ): workflow = REQUEST_GENERATOR_DEFINITION_RUN - cascade_role = CascadeRole.SOURCE - - def output_capture(self, requests: Sequence[RequestGeneratorDefinitionRun]) -> CascadeSourceOutput: - return StubCascadeSourceOutput() def _build_request( self, *, definition: ProposedChangeGeneratorDefinition, target_branch: str, members: list[str] @@ -129,7 +134,6 @@ class ArtifactForcingSelector( ForcingTemplateSelector[ProposedChangeArtifactDefinition, RequestArtifactDefinitionGenerate] ): workflow = REQUEST_ARTIFACT_DEFINITION_GENERATE - cascade_role = CascadeRole.TERMINAL def _build_request( self, *, definition: ProposedChangeArtifactDefinition, target_branch: str, members: list[str] diff --git a/backend/tests/unit/core/merge/selective_regen/definition_selector/conftest.py b/backend/tests/unit/core/merge/selective_regen/definition_selector/conftest.py index f15b957e1b..4635f7c4ed 100644 --- a/backend/tests/unit/core/merge/selective_regen/definition_selector/conftest.py +++ b/backend/tests/unit/core/merge/selective_regen/definition_selector/conftest.py @@ -9,7 +9,6 @@ from infrahub.core.merge.selective_regen.definition_selector.generator_selector import GeneratorSelector from infrahub.core.merge.selective_regen.gate import DefinitionGate from infrahub.core.merge.selective_regen.impacted import ImpactedSubscriberResolver -from tests.helpers.selective_regen import RecordingGeneratorDiffCapturer @pytest.fixture @@ -39,19 +38,11 @@ def artifact_selector( return ArtifactSelector(client=client, gate=gate, impacted_resolver=impacted_resolver, log=log) -@pytest.fixture -def output_capturer() -> RecordingGeneratorDiffCapturer: - return RecordingGeneratorDiffCapturer() - - @pytest.fixture def generator_selector( client: InfrahubClient, gate: DefinitionGate, impacted_resolver: ImpactedSubscriberResolver, log: logging.Logger, - output_capturer: RecordingGeneratorDiffCapturer, ) -> GeneratorSelector: - return GeneratorSelector( - client=client, gate=gate, impacted_resolver=impacted_resolver, log=log, output_capturer=output_capturer - ) + return GeneratorSelector(client=client, gate=gate, impacted_resolver=impacted_resolver, log=log) diff --git a/backend/tests/unit/core/merge/selective_regen/definition_selector/test_artifact_selector.py b/backend/tests/unit/core/merge/selective_regen/definition_selector/test_artifact_selector.py index b54febdc0f..54b65e42a2 100644 --- a/backend/tests/unit/core/merge/selective_regen/definition_selector/test_artifact_selector.py +++ b/backend/tests/unit/core/merge/selective_regen/definition_selector/test_artifact_selector.py @@ -50,11 +50,6 @@ def test_build_request_carries_definition_identity_and_members(artifact_selector assert request.members == ["m1", "m2"] -def test_output_capture_is_none_for_a_terminal(artifact_selector: ArtifactSelector) -> None: - """An artifact selector is a cascade terminal: it produces no output for the cascade to re-read.""" - assert artifact_selector.output_capture([]) is None - - def test_consolidate_merges_requests_for_the_same_definition(artifact_selector: ArtifactSelector) -> None: """A definition selected more than once collapses to one request; member filters union, empty wins.""" consolidated = artifact_selector.consolidate( diff --git a/backend/tests/unit/core/merge/selective_regen/definition_selector/test_generator_selector.py b/backend/tests/unit/core/merge/selective_regen/definition_selector/test_generator_selector.py index 174d5657fc..810ce8ab7f 100644 --- a/backend/tests/unit/core/merge/selective_regen/definition_selector/test_generator_selector.py +++ b/backend/tests/unit/core/merge/selective_regen/definition_selector/test_generator_selector.py @@ -5,12 +5,10 @@ import pytest -from infrahub.core.timestamp import Timestamp from infrahub.generators.models import ProposedChangeGeneratorDefinition if TYPE_CHECKING: from infrahub.core.merge.selective_regen.definition_selector.generator_selector import GeneratorSelector - from tests.helpers.selective_regen import RecordingGeneratorDiffCapturer TARGET_BRANCH = "main" @@ -46,27 +44,6 @@ def test_build_request_threads_branch_definition_and_members(generator_selector: assert request.target_members == ["m1", "m2"] -async def test_output_capture_captures_the_runs_generators_by_name( - generator_selector: GeneratorSelector, output_capturer: RecordingGeneratorDiffCapturer -) -> None: - """output_capture derives the capture scope from each run's generator name, with no follow-up narrowing.""" - runs = [ - generator_selector._build_request( - definition=_generator_definition(name="gen-a"), target_branch=TARGET_BRANCH, members=[] - ), - generator_selector._build_request( - definition=_generator_definition(name="gen-b"), target_branch=TARGET_BRANCH, members=[] - ), - ] - - output = generator_selector.output_capture(runs) - since = Timestamp() - captured = await output.capture(since=since) - - assert output_capturer.calls == [(since, ["gen-a", "gen-b"])] - assert captured is output_capturer.result - - def test_consolidate_returns_the_runs_unchanged_for_a_source(generator_selector: GeneratorSelector) -> None: """A cascade source is not consolidated; its runs pass through the default unchanged.""" runs = [ diff --git a/backend/tests/unit/core/merge/selective_regen/test_generator_diff_capturer.py b/backend/tests/unit/core/merge/selective_regen/test_generator_output.py similarity index 84% rename from backend/tests/unit/core/merge/selective_regen/test_generator_diff_capturer.py rename to backend/tests/unit/core/merge/selective_regen/test_generator_output.py index 290573dfe3..8502e3415a 100644 --- a/backend/tests/unit/core/merge/selective_regen/test_generator_diff_capturer.py +++ b/backend/tests/unit/core/merge/selective_regen/test_generator_output.py @@ -9,12 +9,14 @@ from infrahub.core.diff.query.filters import EnrichedDiffQueryFilters from infrahub.core.diff.repository.repository import DiffRepository from infrahub.core.diff.summary_serializer import DiffSummarySerializer -from infrahub.core.merge.selective_regen.generator_diff_capturer import ( +from infrahub.core.merge.selective_regen.generator_output import ( CAPTURE_DIFF_NAME_PREFIX, + GeneratorCascadeOutput, GeneratorTrackingGroupDiffCapturer, GeneratorTrackingOutput, ) from infrahub.core.timestamp import Timestamp +from infrahub.generators.models import ProposedChangeGeneratorDefinition, RequestGeneratorDefinitionRun from tests.helpers.selective_regen import RecordingGeneratorDiffCapturer if TYPE_CHECKING: @@ -193,3 +195,37 @@ async def test_generator_tracking_output_forwards_its_names_and_since_to_the_cap assert capturer.calls == [(since, ["gen-a", "gen-b"])] assert captured is capturer.result + + +def _generator_run(name: str) -> RequestGeneratorDefinitionRun: + return RequestGeneratorDefinitionRun( + branch="main", + generator_definition=ProposedChangeGeneratorDefinition( + definition_id="def-1", + definition_name=name, + query_name="q", + convert_query_response=False, + class_name="C", + file_path="gen.py", + group_id="grp-1", + parameters={}, + execute_in_proposed_change=False, + execute_after_merge=True, + query_id="q-1", + query_models=[], + query_payload="query {}", + repository_id="repo-1", + ), + ) + + +async def test_generator_cascade_output_captures_the_runs_generators_by_name() -> None: + """GeneratorCascadeOutput derives the capture scope from each run's generator name.""" + capturer = RecordingGeneratorDiffCapturer() + output = GeneratorCascadeOutput(capturer=capturer).for_requests([_generator_run("gen-a"), _generator_run("gen-b")]) + since = Timestamp() + + captured = await output.capture(since=since) + + assert capturer.calls == [(since, ["gen-a", "gen-b"])] + assert captured is capturer.result diff --git a/backend/tests/unit/core/merge/selective_regen/test_orchestrator.py b/backend/tests/unit/core/merge/selective_regen/test_orchestrator.py index 58b773bd7f..423f400da8 100644 --- a/backend/tests/unit/core/merge/selective_regen/test_orchestrator.py +++ b/backend/tests/unit/core/merge/selective_regen/test_orchestrator.py @@ -2,8 +2,6 @@ from typing import TYPE_CHECKING -import pytest - from infrahub.core.merge.selective_regen.definition_selector.base import DefinitionSelectorBase from infrahub.core.merge.selective_regen.models import ( CascadeRole, @@ -13,19 +11,24 @@ SelectiveRegenerationPlan, ) from infrahub.core.merge.selective_regen.orchestrator import MergeSelectiveRegeneration +from infrahub.core.merge.selective_regen.participant import CascadeSource, CascadeTerminal from infrahub.generators.models import ProposedChangeGeneratorDefinition, RequestGeneratorDefinitionRun from infrahub.git.models import RequestArtifactDefinitionGenerate from infrahub.message_bus.types import ProposedChangeArtifactDefinition from infrahub.workflows.catalogue import REQUEST_ARTIFACT_DEFINITION_GENERATE, REQUEST_GENERATOR_DEFINITION_RUN from tests.helpers.diff_summary import node_diff -from tests.helpers.selective_regen import ArtifactForcingSelector, GeneratorForcingSelector, StubCascadeSourceOutput +from tests.helpers.selective_regen import ( + ArtifactForcingSelector, + GeneratorForcingSelector, + StubCascadeOutput, + StubCascadeSourceOutput, +) if TYPE_CHECKING: from collections.abc import Sequence from infrahub_sdk.diff import NodeDiff - from infrahub.core.merge.selective_regen.models import CascadeSourceOutput from infrahub.core.regeneration.models import RegenerationTrigger from infrahub.workflows.models import WorkflowDefinition @@ -36,24 +39,12 @@ class _RecordingSelector[DefinitionT: DefinitionModel, RequestT](DefinitionSelectorBase[DefinitionT, RequestT]): """A selector that returns a canned list and records the arguments select was called with.""" - def __init__( - self, - result: list[RequestT], - *, - workflow: WorkflowDefinition, - cascade_role: CascadeRole, - output: CascadeSourceOutput | None = None, - ) -> None: + def __init__(self, result: list[RequestT], *, workflow: WorkflowDefinition) -> None: self.result = result self.workflow = workflow - self.cascade_role = cascade_role - self._output = output self.calls: list[tuple[list[NodeDiff], str, list[str]]] = [] self.consolidate_calls: list[list[RequestT]] = [] - def output_capture(self, requests: Sequence[RequestT]) -> CascadeSourceOutput | None: - return self._output - def consolidate(self, requests: Sequence[RequestT]) -> Sequence[RequestT]: self.consolidate_calls.append(list(requests)) return requests @@ -105,51 +96,8 @@ def test_for_role_returns_the_entries_playing_that_role_in_order() -> None: assert plan.for_role(CascadeRole.TERMINAL) == [first_terminal, second_terminal] -async def test_build_plan_rejects_a_cascade_source_without_an_output_capture() -> None: - """A SOURCE that produces no output capture would silently skip the cascade, so the plan is rejected. - - Failing fast at plan construction turns a misconfigured new source into a caught error and safe full - regeneration, rather than a source that runs but whose terminals are never reselected. - """ - source_without_output = _RecordingSelector[ProposedChangeGeneratorDefinition, RequestGeneratorDefinitionRun]( - result=[], workflow=REQUEST_GENERATOR_DEFINITION_RUN, cascade_role=CascadeRole.SOURCE, output=None - ) - - with pytest.raises( - ValueError, - match=( - r"^cascade source 'request-generator-definition-run' produced no output capture; " - r"a source must capture the output its terminals reselect from$" - ), - ): - await MergeSelectiveRegeneration(selectors=[source_without_output]).build_plan( - diff_summary=[], target_branch=TARGET_BRANCH - ) - - -async def test_build_plan_rejects_a_cascade_terminal_carrying_an_output_capture() -> None: - """A TERMINAL that captures output is a wiring error, since only a source feeds the cascade.""" - terminal_with_output = _RecordingSelector[ProposedChangeArtifactDefinition, RequestArtifactDefinitionGenerate]( - result=[], - workflow=REQUEST_ARTIFACT_DEFINITION_GENERATE, - cascade_role=CascadeRole.TERMINAL, - output=StubCascadeSourceOutput(), - ) - - with pytest.raises( - ValueError, - match=( - r"^cascade terminal 'request_artifact_definitions_generate' produced an output capture; " - r"only a source feeds the cascade$" - ), - ): - await MergeSelectiveRegeneration(selectors=[terminal_with_output]).build_plan( - diff_summary=[], target_branch=TARGET_BRANCH - ) - - async def test_build_plan_shares_modified_kinds_and_assembles_plan() -> None: - """build_plan computes the modified kinds once and returns one entry per selector, in order.""" + """build_plan computes the modified kinds once and returns one entry per participant, in order.""" diff_summary = [ _node_diff(node_id="n1", kind="TestDevice"), _node_diff(node_id="n2", kind="TestSite"), @@ -161,17 +109,18 @@ async def test_build_plan_shares_modified_kinds_and_assembles_plan() -> None: ) generator_output = StubCascadeSourceOutput() generator_selector = _RecordingSelector[ProposedChangeGeneratorDefinition, RequestGeneratorDefinitionRun]( - result=[], workflow=REQUEST_GENERATOR_DEFINITION_RUN, cascade_role=CascadeRole.SOURCE, output=generator_output + result=[], workflow=REQUEST_GENERATOR_DEFINITION_RUN ) artifact_selector = _RecordingSelector[ProposedChangeArtifactDefinition, RequestArtifactDefinitionGenerate]( - result=[artifact_request], - workflow=REQUEST_ARTIFACT_DEFINITION_GENERATE, - cascade_role=CascadeRole.TERMINAL, + result=[artifact_request], workflow=REQUEST_ARTIFACT_DEFINITION_GENERATE ) - plan = await MergeSelectiveRegeneration(selectors=[generator_selector, artifact_selector]).build_plan( - diff_summary=diff_summary, target_branch=TARGET_BRANCH - ) + plan = await MergeSelectiveRegeneration( + participants=[ + CascadeSource(generator_selector, output=StubCascadeOutput(result=generator_output)), + CascadeTerminal(artifact_selector), + ] + ).build_plan(diff_summary=diff_summary, target_branch=TARGET_BRANCH) assert [(entry.workflow, entry.cascade_role, entry.requests, entry.output) for entry in plan.entries] == [ (REQUEST_GENERATOR_DEFINITION_RUN, CascadeRole.SOURCE, [], generator_output), @@ -189,7 +138,7 @@ async def test_build_plan_shares_modified_kinds_and_assembles_plan() -> None: async def test_reselect_from_cascade_output_excludes_cascade_sources() -> None: - """The diff of a cascade source's own writes re-runs only the non-source selectors. + """The diff of a cascade source's own writes re-runs only the non-source participants. Re-running a source on the diff it produced would repeat a run already completed. """ @@ -198,14 +147,17 @@ async def test_reselect_from_cascade_output_excludes_cascade_sources() -> None: artifact_definition_id="art-1", artifact_definition_name="art", branch=TARGET_BRANCH, members=["m1"] ) generator_selector = _RecordingSelector[ProposedChangeGeneratorDefinition, RequestGeneratorDefinitionRun]( - result=[], workflow=REQUEST_GENERATOR_DEFINITION_RUN, cascade_role=CascadeRole.SOURCE + result=[], workflow=REQUEST_GENERATOR_DEFINITION_RUN ) artifact_selector = _RecordingSelector[ProposedChangeArtifactDefinition, RequestArtifactDefinitionGenerate]( - result=[artifact_request], workflow=REQUEST_ARTIFACT_DEFINITION_GENERATE, cascade_role=CascadeRole.TERMINAL + result=[artifact_request], workflow=REQUEST_ARTIFACT_DEFINITION_GENERATE ) entries = await MergeSelectiveRegeneration( - selectors=[generator_selector, artifact_selector] + participants=[ + CascadeSource(generator_selector, output=StubCascadeOutput()), + CascadeTerminal(artifact_selector), + ] ).reselect_from_cascade_output(diff_summary=diff_summary, target_branch=TARGET_BRANCH) assert generator_selector.calls == [] @@ -218,7 +170,7 @@ async def test_reselect_from_cascade_output_excludes_cascade_sources() -> None: async def test_reselect_from_cascade_output_escalates_across_terminals_sharing_a_repository() -> None: """A null-fingerprint terminal escalates its whole repository, including a sibling terminal. - The missing-fingerprint set is aggregated over every non-source selector before selection, so it + The missing-fingerprint set is aggregated over every non-source participant before selection, so it keeps the repository-wide fallback build_plan applies rather than escalating each kind in isolation. """ unpopulated_terminal = ArtifactForcingSelector( @@ -229,7 +181,7 @@ async def test_reselect_from_cascade_output_escalates_across_terminals_sharing_a ) entries = await MergeSelectiveRegeneration( - selectors=[unpopulated_terminal, populated_terminal] + participants=[CascadeTerminal(unpopulated_terminal), CascadeTerminal(populated_terminal)] ).reselect_from_cascade_output(diff_summary=[], target_branch=TARGET_BRANCH) assert [len(entry.requests) for entry in entries] == [1, 1] @@ -238,10 +190,10 @@ async def test_reselect_from_cascade_output_escalates_across_terminals_sharing_a async def test_consolidate_submissions_routes_each_workflow_to_its_selector() -> None: """Each entry's requests are consolidated by the selector that owns its workflow, then tagged back.""" generator_selector = _RecordingSelector[ProposedChangeGeneratorDefinition, RequestGeneratorDefinitionRun]( - result=[], workflow=REQUEST_GENERATOR_DEFINITION_RUN, cascade_role=CascadeRole.SOURCE + result=[], workflow=REQUEST_GENERATOR_DEFINITION_RUN ) artifact_selector = _RecordingSelector[ProposedChangeArtifactDefinition, RequestArtifactDefinitionGenerate]( - result=[], workflow=REQUEST_ARTIFACT_DEFINITION_GENERATE, cascade_role=CascadeRole.TERMINAL + result=[], workflow=REQUEST_ARTIFACT_DEFINITION_GENERATE ) generator_run = RequestGeneratorDefinitionRun( branch=TARGET_BRANCH, generator_definition=_generator(fingerprint="fp") @@ -260,9 +212,12 @@ async def test_consolidate_submissions_routes_each_workflow_to_its_selector() -> ), ] - result = MergeSelectiveRegeneration(selectors=[generator_selector, artifact_selector]).consolidate_submissions( - entries - ) + result = MergeSelectiveRegeneration( + participants=[ + CascadeSource(generator_selector, output=StubCascadeOutput()), + CascadeTerminal(artifact_selector), + ] + ).consolidate_submissions(entries) assert generator_selector.consolidate_calls == [[generator_run]] assert artifact_selector.consolidate_calls == [[artifact_request]] @@ -330,9 +285,12 @@ async def test_missing_generator_fingerprint_escalates_a_sibling_artifact_in_the subscriber_by_member={"m1": "s1", "m2": "s2"}, ) - plan = await MergeSelectiveRegeneration(selectors=[generator_selector, artifact_selector]).build_plan( - diff_summary=[], target_branch=TARGET_BRANCH - ) + plan = await MergeSelectiveRegeneration( + participants=[ + CascadeSource(generator_selector, output=StubCascadeOutput()), + CascadeTerminal(artifact_selector), + ] + ).build_plan(diff_summary=[], target_branch=TARGET_BRANCH) generator_entries = plan.for_role(CascadeRole.SOURCE) artifact_entries = plan.for_role(CascadeRole.TERMINAL) diff --git a/backend/tests/unit/core/merge/selective_regen/test_participant.py b/backend/tests/unit/core/merge/selective_regen/test_participant.py new file mode 100644 index 0000000000..f36886ffcf --- /dev/null +++ b/backend/tests/unit/core/merge/selective_regen/test_participant.py @@ -0,0 +1,55 @@ +from __future__ import annotations + +from typing import TYPE_CHECKING + +from infrahub.core.merge.selective_regen.models import CascadeRole +from infrahub.core.merge.selective_regen.participant import CascadeSource, CascadeTerminal +from infrahub.workflows.catalogue import REQUEST_ARTIFACT_DEFINITION_GENERATE, REQUEST_GENERATOR_DEFINITION_RUN +from tests.helpers.selective_regen import ArtifactForcingSelector, GeneratorForcingSelector, StubCascadeSourceOutput + +if TYPE_CHECKING: + from collections.abc import Sequence + + from infrahub.core.merge.selective_regen.models import CascadeSourceOutput, RegenerationRequest + + +class _RecordingCascadeOutput: + """A CascadeOutput that records the requests it was asked to build the capture from.""" + + def __init__(self) -> None: + self.seen: list[Sequence[RegenerationRequest]] = [] + self.result = StubCascadeSourceOutput() + + def for_requests(self, requests: Sequence[RegenerationRequest]) -> CascadeSourceOutput: + self.seen.append(requests) + return self.result + + +def test_source_entry_forwards_its_requests_and_carries_the_built_output() -> None: + """A source's entry is tagged SOURCE and carries the capture its output builds from those requests.""" + output = _RecordingCascadeOutput() + selector = GeneratorForcingSelector(definitions=[], member_ids=[], subscriber_by_member={}) + participant = CascadeSource(selector, output=output) + requests: list[RegenerationRequest] = [] + + entry = participant.to_entry(requests) + + assert participant.role is CascadeRole.SOURCE + assert entry.workflow is REQUEST_GENERATOR_DEFINITION_RUN + assert entry.cascade_role is CascadeRole.SOURCE + assert entry.requests is requests + assert output.seen[0] is requests + assert entry.output is output.result + + +def test_terminal_entry_carries_no_output() -> None: + """A terminal's entry is tagged TERMINAL and carries no output, since nothing reselects from it.""" + selector = ArtifactForcingSelector(definitions=[], member_ids=[], subscriber_by_member={}) + participant = CascadeTerminal(selector) + + entry = participant.to_entry([]) + + assert participant.role is CascadeRole.TERMINAL + assert entry.workflow is REQUEST_ARTIFACT_DEFINITION_GENERATE + assert entry.cascade_role is CascadeRole.TERMINAL + assert entry.output is None From c4f84f4bb60768c669284e924131e4517c130a93 Mon Sep 17 00:00:00 2001 From: Pol Michel Date: Wed, 29 Jul 2026 14:29:30 +0200 Subject: [PATCH 09/16] refactor(regeneration): name the output factory and make protocol impls explicit Rename CascadeOutput to CascadeSourceOutputFactory (and GeneratorCascadeOutput to GeneratorOutputFactory) so the producer reads as a factory for a CascadeSourceOutput rather than a variant of it. Have the production implementations inherit their Protocol explicitly (GeneratorTrackingOutput, GeneratorOutputFactory, GeneratorTrackingGroupDiffCapturer, MergeSelectiveRegeneration), so each protocol's implementer is discoverable by grep and IDE and its conformance is checked at the definition rather than only at the use site. Test doubles stay structural. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../core/merge/selective_regen/generator_output.py | 12 ++++++------ .../infrahub/core/merge/selective_regen/models.py | 2 +- .../core/merge/selective_regen/orchestrator.py | 6 +++--- .../core/merge/selective_regen/participant.py | 4 ++-- backend/tests/helpers/selective_regen.py | 4 ++-- .../merge/selective_regen/test_generator_output.py | 6 +++--- .../core/merge/selective_regen/test_orchestrator.py | 10 +++++----- .../core/merge/selective_regen/test_participant.py | 6 +++--- 8 files changed, 25 insertions(+), 25 deletions(-) diff --git a/backend/infrahub/core/merge/selective_regen/generator_output.py b/backend/infrahub/core/merge/selective_regen/generator_output.py index 1e79f11dc7..0d309687d0 100644 --- a/backend/infrahub/core/merge/selective_regen/generator_output.py +++ b/backend/infrahub/core/merge/selective_regen/generator_output.py @@ -8,6 +8,9 @@ from infrahub.core.diff.query.filters import EnrichedDiffQueryFilters from infrahub.core.timestamp import Timestamp +from infrahub.generators.models import RequestGeneratorDefinitionRun + +from .models import CascadeSourceOutput, CascadeSourceOutputFactory if TYPE_CHECKING: from collections.abc import Sequence @@ -19,9 +22,6 @@ from infrahub.core.diff.coordinator import DiffCoordinator from infrahub.core.diff.repository.repository import DiffRepository from infrahub.core.diff.summary_serializer import DiffSummarySerializer - from infrahub.generators.models import RequestGeneratorDefinitionRun - - from .models import CascadeSourceOutput # A generator tracks the nodes it writes into a per-member group named "-". _TRACKING_HASH = re.compile(r"[0-9a-f]{32}$") @@ -42,7 +42,7 @@ class GeneratorMutationDiffCapturer(Protocol): async def capture(self, *, since: Timestamp, generator_definition_names: list[str]) -> list[NodeDiff]: ... -class GeneratorTrackingOutput: +class GeneratorTrackingOutput(CascadeSourceOutput): """The cascade output of a set of generators, captured through their per-member tracking groups. Binds the generators' definition names to the capturer so the follow-up can capture their output @@ -57,7 +57,7 @@ async def capture(self, *, since: Timestamp) -> list[NodeDiff]: return await self._capturer.capture(since=since, generator_definition_names=self._definition_names) -class GeneratorCascadeOutput: +class GeneratorOutputFactory(CascadeSourceOutputFactory[RequestGeneratorDefinitionRun]): """Produces a set of generators' cascade output from the runs selected for them. Owns the generator-specific step of reading the definition names off the runs, so the capturer it @@ -74,7 +74,7 @@ def for_requests(self, requests: Sequence[RequestGeneratorDefinitionRun]) -> Cas ) -class GeneratorTrackingGroupDiffCapturer: +class GeneratorTrackingGroupDiffCapturer(GeneratorMutationDiffCapturer): """Capture a post-merge generator's own writes, scoped to the nodes it tracked. A time-window diff of the branch alone would also carry any concurrent write landing on it while the diff --git a/backend/infrahub/core/merge/selective_regen/models.py b/backend/infrahub/core/merge/selective_regen/models.py index 50be4bb9f8..c24465a596 100644 --- a/backend/infrahub/core/merge/selective_regen/models.py +++ b/backend/infrahub/core/merge/selective_regen/models.py @@ -45,7 +45,7 @@ class CascadeSourceOutput(Protocol): async def capture(self, *, since: Timestamp) -> list[NodeDiff]: ... -class CascadeOutput[RequestT](Protocol): +class CascadeSourceOutputFactory[RequestT](Protocol): """Produces the output capture for a cascade source from the requests it selected. Bound to a source at wiring time and given that source's selected requests, so the capture is diff --git a/backend/infrahub/core/merge/selective_regen/orchestrator.py b/backend/infrahub/core/merge/selective_regen/orchestrator.py index a882bf6b5e..eaa71d9f95 100644 --- a/backend/infrahub/core/merge/selective_regen/orchestrator.py +++ b/backend/infrahub/core/merge/selective_regen/orchestrator.py @@ -8,7 +8,7 @@ from .definition_selector.generator_selector import GeneratorSelector from .fallbacks import repositories_forcing_full_regeneration from .gate import DefinitionGate -from .generator_output import GeneratorCascadeOutput +from .generator_output import GeneratorOutputFactory from .impacted import ImpactedSubscriberResolver from .models import CascadeRole, PlannedRegeneration, SelectiveRegenerationPlan from .participant import CascadeSource, CascadeTerminal @@ -37,7 +37,7 @@ async def reselect_from_cascade_output( def consolidate_submissions(self, entries: Sequence[PlannedRegeneration]) -> list[PlannedRegeneration]: ... -class MergeSelectiveRegeneration: +class MergeSelectiveRegeneration(RegenerationSelector): """Select the definitions a merge changed, narrowed to affected members, across every participant. Runs each injected participant's selector over a single computation of the diff's modified kinds and @@ -134,7 +134,7 @@ def build_merge_selective_regeneration( participants=[ CascadeSource( GeneratorSelector(client=client, gate=gate, impacted_resolver=impacted_resolver, log=log), - output=GeneratorCascadeOutput(capturer=output_capturer), + output=GeneratorOutputFactory(capturer=output_capturer), ), CascadeTerminal(ArtifactSelector(client=client, gate=gate, impacted_resolver=impacted_resolver, log=log)), ] diff --git a/backend/infrahub/core/merge/selective_regen/participant.py b/backend/infrahub/core/merge/selective_regen/participant.py index e1a6a4f9ca..993932df1a 100644 --- a/backend/infrahub/core/merge/selective_regen/participant.py +++ b/backend/infrahub/core/merge/selective_regen/participant.py @@ -9,7 +9,7 @@ from collections.abc import Sequence from .definition_selector.base import DefinitionSelectorBase - from .models import CascadeOutput, CascadeSourceOutput, RegenerationRequest + from .models import CascadeSourceOutput, CascadeSourceOutputFactory, RegenerationRequest class CascadeParticipant(ABC): @@ -43,7 +43,7 @@ class CascadeSource(CascadeParticipant): role = CascadeRole.SOURCE - def __init__(self, selector: DefinitionSelectorBase[Any, Any], *, output: CascadeOutput[Any]) -> None: + def __init__(self, selector: DefinitionSelectorBase[Any, Any], *, output: CascadeSourceOutputFactory[Any]) -> None: super().__init__(selector) self._output = output diff --git a/backend/tests/helpers/selective_regen.py b/backend/tests/helpers/selective_regen.py index 9fef339afd..0420933fa4 100644 --- a/backend/tests/helpers/selective_regen.py +++ b/backend/tests/helpers/selective_regen.py @@ -33,8 +33,8 @@ async def capture(self, *, since: Timestamp) -> list[NodeDiff]: return [] -class StubCascadeOutput: - """A CascadeOutput yielding a fixed capture, for source doubles that only exercise selection.""" +class StubOutputFactory: + """A CascadeSourceOutputFactory yielding a fixed capture, for source doubles that only select.""" def __init__(self, result: CascadeSourceOutput | None = None) -> None: self._result = result if result is not None else StubCascadeSourceOutput() diff --git a/backend/tests/unit/core/merge/selective_regen/test_generator_output.py b/backend/tests/unit/core/merge/selective_regen/test_generator_output.py index 8502e3415a..f2414204fd 100644 --- a/backend/tests/unit/core/merge/selective_regen/test_generator_output.py +++ b/backend/tests/unit/core/merge/selective_regen/test_generator_output.py @@ -11,7 +11,7 @@ from infrahub.core.diff.summary_serializer import DiffSummarySerializer from infrahub.core.merge.selective_regen.generator_output import ( CAPTURE_DIFF_NAME_PREFIX, - GeneratorCascadeOutput, + GeneratorOutputFactory, GeneratorTrackingGroupDiffCapturer, GeneratorTrackingOutput, ) @@ -220,9 +220,9 @@ def _generator_run(name: str) -> RequestGeneratorDefinitionRun: async def test_generator_cascade_output_captures_the_runs_generators_by_name() -> None: - """GeneratorCascadeOutput derives the capture scope from each run's generator name.""" + """GeneratorOutputFactory derives the capture scope from each run's generator name.""" capturer = RecordingGeneratorDiffCapturer() - output = GeneratorCascadeOutput(capturer=capturer).for_requests([_generator_run("gen-a"), _generator_run("gen-b")]) + output = GeneratorOutputFactory(capturer=capturer).for_requests([_generator_run("gen-a"), _generator_run("gen-b")]) since = Timestamp() captured = await output.capture(since=since) diff --git a/backend/tests/unit/core/merge/selective_regen/test_orchestrator.py b/backend/tests/unit/core/merge/selective_regen/test_orchestrator.py index 423f400da8..d90c6263dc 100644 --- a/backend/tests/unit/core/merge/selective_regen/test_orchestrator.py +++ b/backend/tests/unit/core/merge/selective_regen/test_orchestrator.py @@ -20,8 +20,8 @@ from tests.helpers.selective_regen import ( ArtifactForcingSelector, GeneratorForcingSelector, - StubCascadeOutput, StubCascadeSourceOutput, + StubOutputFactory, ) if TYPE_CHECKING: @@ -117,7 +117,7 @@ async def test_build_plan_shares_modified_kinds_and_assembles_plan() -> None: plan = await MergeSelectiveRegeneration( participants=[ - CascadeSource(generator_selector, output=StubCascadeOutput(result=generator_output)), + CascadeSource(generator_selector, output=StubOutputFactory(result=generator_output)), CascadeTerminal(artifact_selector), ] ).build_plan(diff_summary=diff_summary, target_branch=TARGET_BRANCH) @@ -155,7 +155,7 @@ async def test_reselect_from_cascade_output_excludes_cascade_sources() -> None: entries = await MergeSelectiveRegeneration( participants=[ - CascadeSource(generator_selector, output=StubCascadeOutput()), + CascadeSource(generator_selector, output=StubOutputFactory()), CascadeTerminal(artifact_selector), ] ).reselect_from_cascade_output(diff_summary=diff_summary, target_branch=TARGET_BRANCH) @@ -214,7 +214,7 @@ async def test_consolidate_submissions_routes_each_workflow_to_its_selector() -> result = MergeSelectiveRegeneration( participants=[ - CascadeSource(generator_selector, output=StubCascadeOutput()), + CascadeSource(generator_selector, output=StubOutputFactory()), CascadeTerminal(artifact_selector), ] ).consolidate_submissions(entries) @@ -287,7 +287,7 @@ async def test_missing_generator_fingerprint_escalates_a_sibling_artifact_in_the plan = await MergeSelectiveRegeneration( participants=[ - CascadeSource(generator_selector, output=StubCascadeOutput()), + CascadeSource(generator_selector, output=StubOutputFactory()), CascadeTerminal(artifact_selector), ] ).build_plan(diff_summary=[], target_branch=TARGET_BRANCH) diff --git a/backend/tests/unit/core/merge/selective_regen/test_participant.py b/backend/tests/unit/core/merge/selective_regen/test_participant.py index f36886ffcf..59cef17342 100644 --- a/backend/tests/unit/core/merge/selective_regen/test_participant.py +++ b/backend/tests/unit/core/merge/selective_regen/test_participant.py @@ -13,8 +13,8 @@ from infrahub.core.merge.selective_regen.models import CascadeSourceOutput, RegenerationRequest -class _RecordingCascadeOutput: - """A CascadeOutput that records the requests it was asked to build the capture from.""" +class _RecordingOutputFactory: + """A CascadeSourceOutputFactory that records the requests it was asked to build the capture from.""" def __init__(self) -> None: self.seen: list[Sequence[RegenerationRequest]] = [] @@ -27,7 +27,7 @@ def for_requests(self, requests: Sequence[RegenerationRequest]) -> CascadeSource def test_source_entry_forwards_its_requests_and_carries_the_built_output() -> None: """A source's entry is tagged SOURCE and carries the capture its output builds from those requests.""" - output = _RecordingCascadeOutput() + output = _RecordingOutputFactory() selector = GeneratorForcingSelector(definitions=[], member_ids=[], subscriber_by_member={}) participant = CascadeSource(selector, output=output) requests: list[RegenerationRequest] = [] From 58034e7988b177df77b7fdb27fbac84919b3a8fe Mon Sep 17 00:00:00 2001 From: Pol Michel Date: Wed, 29 Jul 2026 14:39:36 +0200 Subject: [PATCH 10/16] refactor(regeneration): parameterize cascade participants by request type A CascadeSource wired to a selector whose request type does not match its output factory was accepted at construction and failed only while building the plan, degrading to the runtime full-regeneration fallback. Parameterize CascadeParticipant / CascadeSource / CascadeTerminal by the selector's request type and require a matching CascadeSourceOutputFactory[RequestT], so a mismatched wiring is a type error at the composition site rather than a runtime fallback. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../merge/selective_regen/orchestrator.py | 6 +++--- .../core/merge/selective_regen/participant.py | 21 +++++++++++-------- .../merge/selective_regen/test_participant.py | 5 +++-- 3 files changed, 18 insertions(+), 14 deletions(-) diff --git a/backend/infrahub/core/merge/selective_regen/orchestrator.py b/backend/infrahub/core/merge/selective_regen/orchestrator.py index eaa71d9f95..6ba0df41c9 100644 --- a/backend/infrahub/core/merge/selective_regen/orchestrator.py +++ b/backend/infrahub/core/merge/selective_regen/orchestrator.py @@ -1,6 +1,6 @@ from __future__ import annotations -from typing import TYPE_CHECKING, Protocol +from typing import TYPE_CHECKING, Any, Protocol from infrahub.proposed_change.branch_diff import get_modified_kinds @@ -45,7 +45,7 @@ class MergeSelectiveRegeneration(RegenerationSelector): dispatch. Adding a definition kind is one new participant in the injected list, with no change here. """ - def __init__(self, participants: Sequence[CascadeParticipant]) -> None: + def __init__(self, participants: Sequence[CascadeParticipant[Any]]) -> None: self.participants = participants async def build_plan(self, diff_summary: list[NodeDiff], target_branch: str) -> SelectiveRegenerationPlan: @@ -65,7 +65,7 @@ async def reselect_from_cascade_output( return await self._plan(participants, diff_summary=diff_summary, target_branch=target_branch) async def _plan( - self, participants: Sequence[CascadeParticipant], *, diff_summary: list[NodeDiff], target_branch: str + self, participants: Sequence[CascadeParticipant[Any]], *, diff_summary: list[NodeDiff], target_branch: str ) -> list[PlannedRegeneration]: modified_kinds = get_modified_kinds(diff_summary=diff_summary, branch=target_branch) loaded_by_participant = [ diff --git a/backend/infrahub/core/merge/selective_regen/participant.py b/backend/infrahub/core/merge/selective_regen/participant.py index 993932df1a..aefb57ae69 100644 --- a/backend/infrahub/core/merge/selective_regen/participant.py +++ b/backend/infrahub/core/merge/selective_regen/participant.py @@ -12,19 +12,20 @@ from .models import CascadeSourceOutput, CascadeSourceOutputFactory, RegenerationRequest -class CascadeParticipant(ABC): +class CascadeParticipant[RequestT: RegenerationRequest](ABC): """A selector paired with its place in the merge regeneration cascade. Selection is the selector's concern; capturing output for the cascade is a separate one. A participant binds the two so a source and a terminal differ by composition rather than a flag. + The request type is carried so a source's output must match the selector it is wired to. """ role: CascadeRole - def __init__(self, selector: DefinitionSelectorBase[Any, Any]) -> None: + def __init__(self, selector: DefinitionSelectorBase[Any, RequestT]) -> None: self.selector = selector - def to_entry(self, requests: Sequence[RegenerationRequest]) -> PlannedRegeneration: + def to_entry(self, requests: Sequence[RequestT]) -> PlannedRegeneration: """Turn this participant's selected requests into a plan entry carrying its cascade output.""" return PlannedRegeneration( workflow=self.selector.workflow, @@ -34,27 +35,29 @@ def to_entry(self, requests: Sequence[RegenerationRequest]) -> PlannedRegenerati ) @abstractmethod - def _capture(self, requests: Sequence[RegenerationRequest]) -> CascadeSourceOutput | None: + def _capture(self, requests: Sequence[RequestT]) -> CascadeSourceOutput | None: """The output capture for these requests, or None when nothing downstream reselects from it.""" -class CascadeSource(CascadeParticipant): +class CascadeSource[RequestT: RegenerationRequest](CascadeParticipant[RequestT]): """A participant whose output the cascade re-reads to reselect the terminals that consume it.""" role = CascadeRole.SOURCE - def __init__(self, selector: DefinitionSelectorBase[Any, Any], *, output: CascadeSourceOutputFactory[Any]) -> None: + def __init__( + self, selector: DefinitionSelectorBase[Any, RequestT], *, output: CascadeSourceOutputFactory[RequestT] + ) -> None: super().__init__(selector) self._output = output - def _capture(self, requests: Sequence[RegenerationRequest]) -> CascadeSourceOutput: + def _capture(self, requests: Sequence[RequestT]) -> CascadeSourceOutput: return self._output.for_requests(requests) -class CascadeTerminal(CascadeParticipant): +class CascadeTerminal[RequestT: RegenerationRequest](CascadeParticipant[RequestT]): """A participant that ends the cascade -- nothing downstream re-reads what it produces.""" role = CascadeRole.TERMINAL - def _capture(self, requests: Sequence[RegenerationRequest]) -> None: # noqa: ARG002 + def _capture(self, requests: Sequence[RequestT]) -> None: # noqa: ARG002 return None diff --git a/backend/tests/unit/core/merge/selective_regen/test_participant.py b/backend/tests/unit/core/merge/selective_regen/test_participant.py index 59cef17342..5769eab3c5 100644 --- a/backend/tests/unit/core/merge/selective_regen/test_participant.py +++ b/backend/tests/unit/core/merge/selective_regen/test_participant.py @@ -4,6 +4,7 @@ from infrahub.core.merge.selective_regen.models import CascadeRole from infrahub.core.merge.selective_regen.participant import CascadeSource, CascadeTerminal +from infrahub.generators.models import RequestGeneratorDefinitionRun from infrahub.workflows.catalogue import REQUEST_ARTIFACT_DEFINITION_GENERATE, REQUEST_GENERATOR_DEFINITION_RUN from tests.helpers.selective_regen import ArtifactForcingSelector, GeneratorForcingSelector, StubCascadeSourceOutput @@ -29,8 +30,8 @@ def test_source_entry_forwards_its_requests_and_carries_the_built_output() -> No """A source's entry is tagged SOURCE and carries the capture its output builds from those requests.""" output = _RecordingOutputFactory() selector = GeneratorForcingSelector(definitions=[], member_ids=[], subscriber_by_member={}) - participant = CascadeSource(selector, output=output) - requests: list[RegenerationRequest] = [] + participant = CascadeSource[RequestGeneratorDefinitionRun](selector, output=output) + requests: list[RequestGeneratorDefinitionRun] = [] entry = participant.to_entry(requests) From 0d6b3b8ca3a86e42c84b3ba61f22995f3ee55f11 Mon Sep 17 00:00:00 2001 From: Pol Michel Date: Wed, 29 Jul 2026 14:47:15 +0200 Subject: [PATCH 11/16] refactor(regeneration): regenerate every terminal on source failure, not only artifacts When a cascade source fails or its output cannot be captured, the follow-up fell back to a blanket regeneration hard-coded to artifacts, so a newly wired terminal kind would be left stale despite the generic terminal contract. Each selector now owns its blanket-regeneration trigger and parameters, a participant exposes its own full regeneration, and the orchestrator aggregates one per terminal participant. The dispatcher's source-failure fallback submits every terminal's blanket regeneration instead of the artifact trigger, so adding a terminal kind stays a wiring-only change on the failure path too. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../core/merge/regeneration_dispatcher.py | 27 ++++++++++--------- .../definition_selector/artifact_selector.py | 3 ++- .../definition_selector/base.py | 8 +++++- .../definition_selector/generator_selector.py | 9 ++++++- .../core/merge/selective_regen/models.py | 14 +++++++++- .../merge/selective_regen/orchestrator.py | 12 ++++++++- .../core/merge/selective_regen/participant.py | 9 ++++++- backend/tests/helpers/selective_regen.py | 9 ++++++- .../test_generator_selector.py | 9 +++++++ .../selective_regen/test_orchestrator.py | 22 ++++++++++++++- .../merge/test_regeneration_dispatcher.py | 5 ++++ 11 files changed, 106 insertions(+), 21 deletions(-) diff --git a/backend/infrahub/core/merge/regeneration_dispatcher.py b/backend/infrahub/core/merge/regeneration_dispatcher.py index a7d55e4710..16b989fa0e 100644 --- a/backend/infrahub/core/merge/regeneration_dispatcher.py +++ b/backend/infrahub/core/merge/regeneration_dispatcher.py @@ -147,16 +147,16 @@ async def _dispatch_plan( self.log.exception("Post-merge generator run failed") if generator_failed: - # A failed generator's consuming artifacts cannot be selected from its output, so regenerate - # every artifact -- but never re-run the generators, which would fail the same way again. - await self._submit_full_artifact_regeneration(context=context, target_branch=target_branch) + # A failed source's consuming terminals cannot be selected from its output, so regenerate + # every terminal -- but never re-run the sources, which would fail the same way again. + await self._submit_full_terminal_regeneration(context=context, target_branch=target_branch) return targeted = await self._reselect_from_cascade_output( context=context, target_branch=target_branch, sources=sources, since=cascade_started_at ) if targeted is None: - # Every artifact was already regenerated wholesale, which covers the merge-diff selection too. + # Every terminal was already regenerated wholesale, which covers the merge-diff selection too. return # Dispatched only after the capture, so the capture window never sees these generations' own writes. await self._submit(context=context, entries=[*terminals, *targeted]) @@ -180,8 +180,8 @@ async def _reselect_from_cascade_output( """Reselect the fire-and-forget generations the just-run sources' own output requires. Each source captures its own output; the terminals that read it are then reselected from the - combined diff. Returns ``None`` after regenerating every artifact wholesale when that output - cannot be captured or selected, so a source's writes can never leave a consuming artifact stale. + combined diff. Returns ``None`` after regenerating every terminal wholesale when that output + cannot be captured or selected, so a source's writes can never leave a consuming terminal stale. """ try: captured: list[NodeDiff] = [] @@ -192,11 +192,11 @@ async def _reselect_from_cascade_output( diff_summary=captured, target_branch=target_branch ) except Exception: - self.log.exception("Failed to target artifacts from generator output; regenerating all artifacts instead") - await self._submit_full_artifact_regeneration(context=context, target_branch=target_branch) + self.log.exception("Failed to target terminals from cascade output; regenerating all terminals instead") + await self._submit_full_terminal_regeneration(context=context, target_branch=target_branch) return None targeted_count = sum(len(entry.requests) for entry in targeted) - self.log.debug(f"Targeted {targeted_count} artifact definition(s) from generator output") + self.log.debug(f"Targeted {targeted_count} terminal definition(s) from cascade output") return targeted async def _full_regeneration( @@ -205,7 +205,8 @@ async def _full_regeneration( self.log.debug(f"{reason}; regenerating all definitions") await submit_full_regeneration(workflow=self.workflow, context=context, target_branch=target_branch) - async def _submit_full_artifact_regeneration(self, context: InfrahubContext, target_branch: str) -> None: - await self.workflow.submit_workflow( - workflow=TRIGGER_ARTIFACT_DEFINITION_GENERATE, context=context, parameters={"branch": target_branch} - ) + async def _submit_full_terminal_regeneration(self, context: InfrahubContext, target_branch: str) -> None: + for regeneration in self.selector.terminal_full_regenerations(target_branch): + await self.workflow.submit_workflow( + workflow=regeneration.workflow, context=context, parameters=regeneration.parameters + ) diff --git a/backend/infrahub/core/merge/selective_regen/definition_selector/artifact_selector.py b/backend/infrahub/core/merge/selective_regen/definition_selector/artifact_selector.py index 6c3b4cfcaa..cc7d98ae3d 100644 --- a/backend/infrahub/core/merge/selective_regen/definition_selector/artifact_selector.py +++ b/backend/infrahub/core/merge/selective_regen/definition_selector/artifact_selector.py @@ -10,7 +10,7 @@ from infrahub.git.models import RequestArtifactDefinitionGenerate from infrahub.git.utils import fetch_artifact_definition_targets from infrahub.message_bus.types import ProposedChangeArtifactDefinition -from infrahub.workflows.catalogue import REQUEST_ARTIFACT_DEFINITION_GENERATE +from infrahub.workflows.catalogue import REQUEST_ARTIFACT_DEFINITION_GENERATE, TRIGGER_ARTIFACT_DEFINITION_GENERATE from ..models import LoadedDefinition from .base import DefinitionSelectorBase @@ -24,6 +24,7 @@ class ArtifactSelector(DefinitionSelectorBase[ProposedChangeArtifactDefinition, subscriber_kind = InfrahubKind.ARTIFACT workflow = REQUEST_ARTIFACT_DEFINITION_GENERATE + full_regeneration_workflow = TRIGGER_ARTIFACT_DEFINITION_GENERATE def consolidate( self, requests: Sequence[RequestArtifactDefinitionGenerate] diff --git a/backend/infrahub/core/merge/selective_regen/definition_selector/base.py b/backend/infrahub/core/merge/selective_regen/definition_selector/base.py index 8764f6122e..ff3af1a91f 100644 --- a/backend/infrahub/core/merge/selective_regen/definition_selector/base.py +++ b/backend/infrahub/core/merge/selective_regen/definition_selector/base.py @@ -1,7 +1,7 @@ from __future__ import annotations from abc import ABC, abstractmethod -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Any from infrahub.core.regeneration.members import map_subscriber_ids_by_member @@ -46,6 +46,8 @@ class DefinitionSelectorBase[DefinitionT: DefinitionModel, RequestT](ABC): subscriber_kind: str workflow: WorkflowDefinition """The workflow that runs this selector's requests.""" + full_regeneration_workflow: WorkflowDefinition + """The workflow that regenerates every definition of this kind, ignoring selective narrowing.""" def __init__( self, @@ -136,6 +138,10 @@ async def select( requests.append(self._build_request(definition=definition, target_branch=target_branch, members=members)) return requests + def full_regeneration_parameters(self, *, target_branch: str) -> dict[str, Any]: + """Parameters for the blanket regeneration of this kind; extended by kinds that need more.""" + return {"branch": target_branch} + def consolidate(self, requests: Sequence[RequestT]) -> Sequence[RequestT]: """Combine this selector's requests before dispatch; by default each is dispatched as-is. diff --git a/backend/infrahub/core/merge/selective_regen/definition_selector/generator_selector.py b/backend/infrahub/core/merge/selective_regen/definition_selector/generator_selector.py index 0a4dac448f..c61d139d06 100644 --- a/backend/infrahub/core/merge/selective_regen/definition_selector/generator_selector.py +++ b/backend/infrahub/core/merge/selective_regen/definition_selector/generator_selector.py @@ -1,16 +1,19 @@ from __future__ import annotations +from typing import Any + from infrahub_sdk.protocols import CoreGeneratorDefinition from infrahub.core.constants import InfrahubKind from infrahub.core.regeneration.members import run_generator +from infrahub.generators.constants import GeneratorDefinitionRunSource from infrahub.generators.models import ( ProposedChangeGeneratorDefinition, RequestGeneratorDefinitionRun, build_generator_definition, ) from infrahub.git.utils import fetch_proposed_change_generator_definition_targets -from infrahub.workflows.catalogue import REQUEST_GENERATOR_DEFINITION_RUN +from infrahub.workflows.catalogue import REQUEST_GENERATOR_DEFINITION_RUN, TRIGGER_GENERATOR_DEFINITION_RUN from ..models import LoadedDefinition from .base import DefinitionSelectorBase @@ -21,6 +24,10 @@ class GeneratorSelector(DefinitionSelectorBase[ProposedChangeGeneratorDefinition subscriber_kind = InfrahubKind.GENERATORINSTANCE workflow = REQUEST_GENERATOR_DEFINITION_RUN + full_regeneration_workflow = TRIGGER_GENERATOR_DEFINITION_RUN + + def full_regeneration_parameters(self, *, target_branch: str) -> dict[str, Any]: + return {"branch": target_branch, "source": GeneratorDefinitionRunSource.MERGE} async def load_definitions( self, *, target_branch: str diff --git a/backend/infrahub/core/merge/selective_regen/models.py b/backend/infrahub/core/merge/selective_regen/models.py index c24465a596..1428138d1b 100644 --- a/backend/infrahub/core/merge/selective_regen/models.py +++ b/backend/infrahub/core/merge/selective_regen/models.py @@ -2,7 +2,7 @@ from dataclasses import dataclass from enum import Enum -from typing import TYPE_CHECKING, Protocol +from typing import TYPE_CHECKING, Any, Protocol if TYPE_CHECKING: from collections.abc import Sequence @@ -85,6 +85,18 @@ def for_role(self, cascade_role: CascadeRole) -> list[PlannedRegeneration]: return [entry for entry in self.entries if entry.cascade_role is cascade_role] +@dataclass(frozen=True) +class FullRegeneration: + """A blanket regeneration submission for one kind, ignoring any selective narrowing. + + Dispatched when a cascade source's output is unavailable, so every definition of a terminal kind + is regenerated rather than only the ones the diff would have selected. + """ + + workflow: WorkflowDefinition + parameters: dict[str, Any] + + @dataclass(frozen=True) class GateResult: """The outcome of the definition-level selection gate. diff --git a/backend/infrahub/core/merge/selective_regen/orchestrator.py b/backend/infrahub/core/merge/selective_regen/orchestrator.py index 6ba0df41c9..db7f7afde3 100644 --- a/backend/infrahub/core/merge/selective_regen/orchestrator.py +++ b/backend/infrahub/core/merge/selective_regen/orchestrator.py @@ -21,7 +21,7 @@ from infrahub_sdk.diff import NodeDiff from .generator_output import GeneratorMutationDiffCapturer - from .models import DefinitionModel, RegenerationRequest + from .models import DefinitionModel, FullRegeneration, RegenerationRequest from .participant import CascadeParticipant @@ -36,6 +36,8 @@ async def reselect_from_cascade_output( def consolidate_submissions(self, entries: Sequence[PlannedRegeneration]) -> list[PlannedRegeneration]: ... + def terminal_full_regenerations(self, target_branch: str) -> list[FullRegeneration]: ... + class MergeSelectiveRegeneration(RegenerationSelector): """Select the definitions a merge changed, narrowed to affected members, across every participant. @@ -115,6 +117,14 @@ def consolidate_submissions(self, entries: Sequence[PlannedRegeneration]) -> lis ) return submissions + def terminal_full_regenerations(self, target_branch: str) -> list[FullRegeneration]: + """The blanket regenerations for every terminal participant, for the source-failure fallback.""" + return [ + participant.full_regeneration(target_branch=target_branch) + for participant in self.participants + if participant.role is CascadeRole.TERMINAL + ] + def build_merge_selective_regeneration( *, diff --git a/backend/infrahub/core/merge/selective_regen/participant.py b/backend/infrahub/core/merge/selective_regen/participant.py index aefb57ae69..5e5affbafc 100644 --- a/backend/infrahub/core/merge/selective_regen/participant.py +++ b/backend/infrahub/core/merge/selective_regen/participant.py @@ -3,7 +3,7 @@ from abc import ABC, abstractmethod from typing import TYPE_CHECKING, Any -from .models import CascadeRole, PlannedRegeneration +from .models import CascadeRole, FullRegeneration, PlannedRegeneration if TYPE_CHECKING: from collections.abc import Sequence @@ -34,6 +34,13 @@ def to_entry(self, requests: Sequence[RequestT]) -> PlannedRegeneration: output=self._capture(requests), ) + def full_regeneration(self, *, target_branch: str) -> FullRegeneration: + """The blanket regeneration of this participant's kind, for when selective output is unavailable.""" + return FullRegeneration( + workflow=self.selector.full_regeneration_workflow, + parameters=self.selector.full_regeneration_parameters(target_branch=target_branch), + ) + @abstractmethod def _capture(self, requests: Sequence[RequestT]) -> CascadeSourceOutput | None: """The output capture for these requests, or None when nothing downstream reselects from it.""" diff --git a/backend/tests/helpers/selective_regen.py b/backend/tests/helpers/selective_regen.py index 0420933fa4..06771a38d9 100644 --- a/backend/tests/helpers/selective_regen.py +++ b/backend/tests/helpers/selective_regen.py @@ -15,7 +15,12 @@ from infrahub.generators.models import ProposedChangeGeneratorDefinition, RequestGeneratorDefinitionRun from infrahub.git.models import RequestArtifactDefinitionGenerate from infrahub.message_bus.types import ProposedChangeArtifactDefinition -from infrahub.workflows.catalogue import REQUEST_ARTIFACT_DEFINITION_GENERATE, REQUEST_GENERATOR_DEFINITION_RUN +from infrahub.workflows.catalogue import ( + REQUEST_ARTIFACT_DEFINITION_GENERATE, + REQUEST_GENERATOR_DEFINITION_RUN, + TRIGGER_ARTIFACT_DEFINITION_GENERATE, + TRIGGER_GENERATOR_DEFINITION_RUN, +) if TYPE_CHECKING: from collections.abc import Sequence @@ -121,6 +126,7 @@ class GeneratorForcingSelector( ForcingTemplateSelector[ProposedChangeGeneratorDefinition, RequestGeneratorDefinitionRun] ): workflow = REQUEST_GENERATOR_DEFINITION_RUN + full_regeneration_workflow = TRIGGER_GENERATOR_DEFINITION_RUN def _build_request( self, *, definition: ProposedChangeGeneratorDefinition, target_branch: str, members: list[str] @@ -134,6 +140,7 @@ class ArtifactForcingSelector( ForcingTemplateSelector[ProposedChangeArtifactDefinition, RequestArtifactDefinitionGenerate] ): workflow = REQUEST_ARTIFACT_DEFINITION_GENERATE + full_regeneration_workflow = TRIGGER_ARTIFACT_DEFINITION_GENERATE def _build_request( self, *, definition: ProposedChangeArtifactDefinition, target_branch: str, members: list[str] diff --git a/backend/tests/unit/core/merge/selective_regen/definition_selector/test_generator_selector.py b/backend/tests/unit/core/merge/selective_regen/definition_selector/test_generator_selector.py index 810ce8ab7f..e9adff631c 100644 --- a/backend/tests/unit/core/merge/selective_regen/definition_selector/test_generator_selector.py +++ b/backend/tests/unit/core/merge/selective_regen/definition_selector/test_generator_selector.py @@ -5,6 +5,7 @@ import pytest +from infrahub.generators.constants import GeneratorDefinitionRunSource from infrahub.generators.models import ProposedChangeGeneratorDefinition if TYPE_CHECKING: @@ -44,6 +45,14 @@ def test_build_request_threads_branch_definition_and_members(generator_selector: assert request.target_members == ["m1", "m2"] +def test_full_regeneration_parameters_carry_the_merge_source(generator_selector: GeneratorSelector) -> None: + """A generator's blanket regeneration is tagged as a merge-sourced run.""" + assert generator_selector.full_regeneration_parameters(target_branch=TARGET_BRANCH) == { + "branch": TARGET_BRANCH, + "source": GeneratorDefinitionRunSource.MERGE, + } + + def test_consolidate_returns_the_runs_unchanged_for_a_source(generator_selector: GeneratorSelector) -> None: """A cascade source is not consolidated; its runs pass through the default unchanged.""" runs = [ diff --git a/backend/tests/unit/core/merge/selective_regen/test_orchestrator.py b/backend/tests/unit/core/merge/selective_regen/test_orchestrator.py index d90c6263dc..4f7a4cd414 100644 --- a/backend/tests/unit/core/merge/selective_regen/test_orchestrator.py +++ b/backend/tests/unit/core/merge/selective_regen/test_orchestrator.py @@ -15,7 +15,11 @@ from infrahub.generators.models import ProposedChangeGeneratorDefinition, RequestGeneratorDefinitionRun from infrahub.git.models import RequestArtifactDefinitionGenerate from infrahub.message_bus.types import ProposedChangeArtifactDefinition -from infrahub.workflows.catalogue import REQUEST_ARTIFACT_DEFINITION_GENERATE, REQUEST_GENERATOR_DEFINITION_RUN +from infrahub.workflows.catalogue import ( + REQUEST_ARTIFACT_DEFINITION_GENERATE, + REQUEST_GENERATOR_DEFINITION_RUN, + TRIGGER_ARTIFACT_DEFINITION_GENERATE, +) from tests.helpers.diff_summary import node_diff from tests.helpers.selective_regen import ( ArtifactForcingSelector, @@ -227,6 +231,22 @@ async def test_consolidate_submissions_routes_each_workflow_to_its_selector() -> ] +def test_terminal_full_regenerations_cover_every_terminal_and_exclude_sources() -> None: + """The source-failure fallback regenerates every terminal kind wholesale, never the sources.""" + source = CascadeSource[RequestGeneratorDefinitionRun]( + GeneratorForcingSelector(definitions=[], member_ids=[], subscriber_by_member={}), output=StubOutputFactory() + ) + terminal = CascadeTerminal(ArtifactForcingSelector(definitions=[], member_ids=[], subscriber_by_member={})) + + regenerations = MergeSelectiveRegeneration(participants=[source, terminal]).terminal_full_regenerations( + TARGET_BRANCH + ) + + assert [(regeneration.workflow, regeneration.parameters) for regeneration in regenerations] == [ + (TRIGGER_ARTIFACT_DEFINITION_GENERATE, {"branch": TARGET_BRANCH}), + ] + + def _generator(*, fingerprint: str | None) -> ProposedChangeGeneratorDefinition: return ProposedChangeGeneratorDefinition( definition_id="gen-def", diff --git a/backend/tests/unit/core/merge/test_regeneration_dispatcher.py b/backend/tests/unit/core/merge/test_regeneration_dispatcher.py index 4dc151f5a0..3f2efa1d2c 100644 --- a/backend/tests/unit/core/merge/test_regeneration_dispatcher.py +++ b/backend/tests/unit/core/merge/test_regeneration_dispatcher.py @@ -14,6 +14,7 @@ from infrahub.core.merge.regeneration_dispatcher import PostMergeRegenerationDispatcher from infrahub.core.merge.selective_regen.models import ( CascadeRole, + FullRegeneration, PlannedRegeneration, SelectiveRegenerationPlan, ) @@ -107,6 +108,10 @@ def consolidate_submissions(self, entries: Sequence[PlannedRegeneration]) -> lis """Return the canned submissions when set, otherwise the entries unchanged.""" return self._submissions if self._submissions is not None else list(entries) + def terminal_full_regenerations(self, target_branch: str) -> list[FullRegeneration]: + """The blanket regeneration a single artifact terminal would contribute.""" + return [FullRegeneration(workflow=TRIGGER_ARTIFACT_DEFINITION_GENERATE, parameters={"branch": target_branch})] + class _FakeSourceOutput: """A CascadeSourceOutput returning a canned diff or raising, recording its capture calls.""" From 0966e88f7dc126953259bdf32f58702b79efdf79 Mon Sep 17 00:00:00 2001 From: Pol Michel Date: Wed, 29 Jul 2026 14:58:10 +0200 Subject: [PATCH 12/16] refactor(regeneration): drive participants through their own API, not their selector The orchestrator reached through participant.selector to load, select, read the workflow, and consolidate -- a Law-of-Demeter violation that leaked the participant's selector as public state. Make the selector private and give the participant the operations the follow-up needs (load, plan, workflow, consolidated_entry), each delegating to the selector internally, so the orchestrator talks only to the participant. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../merge/selective_regen/orchestrator.py | 35 +++++-------- .../core/merge/selective_regen/participant.py | 52 ++++++++++++++++--- 2 files changed, 59 insertions(+), 28 deletions(-) diff --git a/backend/infrahub/core/merge/selective_regen/orchestrator.py b/backend/infrahub/core/merge/selective_regen/orchestrator.py index db7f7afde3..9afa25a14d 100644 --- a/backend/infrahub/core/merge/selective_regen/orchestrator.py +++ b/backend/infrahub/core/merge/selective_regen/orchestrator.py @@ -71,8 +71,7 @@ async def _plan( ) -> list[PlannedRegeneration]: modified_kinds = get_modified_kinds(diff_summary=diff_summary, branch=target_branch) loaded_by_participant = [ - (participant, await participant.selector.load_definitions(target_branch=target_branch)) - for participant in participants + (participant, await participant.load(target_branch=target_branch)) for participant in participants ] # Aggregated over every participant's definitions so a repository escalated by any missing @@ -84,14 +83,15 @@ async def _plan( entries: list[PlannedRegeneration] = [] for participant, loaded_definitions in loaded_by_participant: - requests = await participant.selector.select( - loaded_definitions=loaded_definitions, - forced_repositories=forced_repositories, - diff_summary=diff_summary, - target_branch=target_branch, - modified_kinds=modified_kinds, + entries.append( + await participant.plan( + loaded_definitions=loaded_definitions, + forced_repositories=forced_repositories, + diff_summary=diff_summary, + target_branch=target_branch, + modified_kinds=modified_kinds, + ) ) - entries.append(participant.to_entry(requests)) return entries def consolidate_submissions(self, entries: Sequence[PlannedRegeneration]) -> list[PlannedRegeneration]: @@ -101,21 +101,14 @@ def consolidate_submissions(self, entries: Sequence[PlannedRegeneration]) -> lis artifact selected by both the merge diff and a generator's output collapses to a single request -- without the follow-up knowing how to merge them. """ - participant_by_workflow = {participant.selector.workflow.name: participant for participant in self.participants} + participant_by_workflow = {participant.workflow.name: participant for participant in self.participants} requests_by_workflow: dict[str, list[RegenerationRequest]] = {} for entry in entries: requests_by_workflow.setdefault(entry.workflow.name, []).extend(entry.requests) - submissions: list[PlannedRegeneration] = [] - for workflow_name, requests in requests_by_workflow.items(): - participant = participant_by_workflow[workflow_name] - submissions.append( - PlannedRegeneration( - workflow=participant.selector.workflow, - cascade_role=participant.role, - requests=participant.selector.consolidate(requests), - ) - ) - return submissions + return [ + participant_by_workflow[workflow_name].consolidated_entry(requests) + for workflow_name, requests in requests_by_workflow.items() + ] def terminal_full_regenerations(self, target_branch: str) -> list[FullRegeneration]: """The blanket regenerations for every terminal participant, for the source-failure fallback.""" diff --git a/backend/infrahub/core/merge/selective_regen/participant.py b/backend/infrahub/core/merge/selective_regen/participant.py index 5e5affbafc..b1feafa7b9 100644 --- a/backend/infrahub/core/merge/selective_regen/participant.py +++ b/backend/infrahub/core/merge/selective_regen/participant.py @@ -8,37 +8,75 @@ if TYPE_CHECKING: from collections.abc import Sequence + from infrahub_sdk.diff import NodeDiff + + from infrahub.core.regeneration.models import RegenerationTrigger + from infrahub.workflows.models import WorkflowDefinition + from .definition_selector.base import DefinitionSelectorBase - from .models import CascadeSourceOutput, CascadeSourceOutputFactory, RegenerationRequest + from .models import CascadeSourceOutput, CascadeSourceOutputFactory, LoadedDefinition, RegenerationRequest class CascadeParticipant[RequestT: RegenerationRequest](ABC): """A selector paired with its place in the merge regeneration cascade. Selection is the selector's concern; capturing output for the cascade is a separate one. A - participant binds the two so a source and a terminal differ by composition rather than a flag. - The request type is carried so a source's output must match the selector it is wired to. + participant binds the two so a source and a terminal differ by composition rather than a flag, + and is the single surface the follow-up drives -- it delegates to its selector rather than + exposing it. The request type is carried so a source's output must match the selector it wraps. """ role: CascadeRole def __init__(self, selector: DefinitionSelectorBase[Any, RequestT]) -> None: - self.selector = selector + self._selector = selector + + @property + def workflow(self) -> WorkflowDefinition: + """The workflow that runs this participant's requests.""" + return self._selector.workflow + + async def load(self, *, target_branch: str) -> list[LoadedDefinition[Any]]: + """Load this participant's candidate definitions, each paired with its target group id.""" + return await self._selector.load_definitions(target_branch=target_branch) + + async def plan( + self, + *, + loaded_definitions: list[LoadedDefinition[Any]], + forced_repositories: dict[str, RegenerationTrigger], + diff_summary: list[NodeDiff], + target_branch: str, + modified_kinds: list[str], + ) -> PlannedRegeneration: + """Select from the loaded definitions and turn the result into this participant's plan entry.""" + requests = await self._selector.select( + loaded_definitions=loaded_definitions, + forced_repositories=forced_repositories, + diff_summary=diff_summary, + target_branch=target_branch, + modified_kinds=modified_kinds, + ) + return self.to_entry(requests) def to_entry(self, requests: Sequence[RequestT]) -> PlannedRegeneration: """Turn this participant's selected requests into a plan entry carrying its cascade output.""" return PlannedRegeneration( - workflow=self.selector.workflow, + workflow=self.workflow, cascade_role=self.role, requests=requests, output=self._capture(requests), ) + def consolidated_entry(self, requests: Sequence[RequestT]) -> PlannedRegeneration: + """Collapse requests that target the same definition into a single dispatchable entry.""" + return self.to_entry(self._selector.consolidate(requests)) + def full_regeneration(self, *, target_branch: str) -> FullRegeneration: """The blanket regeneration of this participant's kind, for when selective output is unavailable.""" return FullRegeneration( - workflow=self.selector.full_regeneration_workflow, - parameters=self.selector.full_regeneration_parameters(target_branch=target_branch), + workflow=self._selector.full_regeneration_workflow, + parameters=self._selector.full_regeneration_parameters(target_branch=target_branch), ) @abstractmethod From 891d1b60d3c21044728242c2da8f7e7375b7e721 Mon Sep 17 00:00:00 2001 From: Pol Michel Date: Wed, 29 Jul 2026 15:08:11 +0200 Subject: [PATCH 13/16] docs(regeneration): document the RegenerationSelector protocol methods Co-Authored-By: Claude Opus 4.8 (1M context) --- .../core/merge/selective_regen/orchestrator.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/backend/infrahub/core/merge/selective_regen/orchestrator.py b/backend/infrahub/core/merge/selective_regen/orchestrator.py index 9afa25a14d..6b5fc6a605 100644 --- a/backend/infrahub/core/merge/selective_regen/orchestrator.py +++ b/backend/infrahub/core/merge/selective_regen/orchestrator.py @@ -28,15 +28,23 @@ class RegenerationSelector(Protocol): """Computes which definitions and members a merge diff requires be regenerated.""" - async def build_plan(self, diff_summary: list[NodeDiff], target_branch: str) -> SelectiveRegenerationPlan: ... + async def build_plan(self, diff_summary: list[NodeDiff], target_branch: str) -> SelectiveRegenerationPlan: + """Return the plan for a merge diff: one entry per participant, tagged with its cascade role.""" + ... async def reselect_from_cascade_output( self, diff_summary: list[NodeDiff], target_branch: str - ) -> list[PlannedRegeneration]: ... + ) -> list[PlannedRegeneration]: + """Re-select the entries a cascade source's captured output requires, excluding the sources.""" + ... - def consolidate_submissions(self, entries: Sequence[PlannedRegeneration]) -> list[PlannedRegeneration]: ... + def consolidate_submissions(self, entries: Sequence[PlannedRegeneration]) -> list[PlannedRegeneration]: + """Collapse the entries into one dispatchable entry per workflow, deduped by their owner.""" + ... - def terminal_full_regenerations(self, target_branch: str) -> list[FullRegeneration]: ... + def terminal_full_regenerations(self, target_branch: str) -> list[FullRegeneration]: + """The blanket regeneration for every terminal, for when a source's output is unavailable.""" + ... class MergeSelectiveRegeneration(RegenerationSelector): From 35f13db1fbb647d55cf7c1fa598fcfeea319a1cd Mon Sep 17 00:00:00 2001 From: Pol Michel Date: Wed, 29 Jul 2026 15:17:53 +0200 Subject: [PATCH 14/16] refactor(regeneration): rename the dispatcher's plan collaborator to planner The dispatcher held its plan-producing collaborator as `selector`, which collided with the per-kind DefinitionSelectorBase selectors it actually coordinates and undersold its plan-shaping and fallback duties (consolidate_submissions, terminal_full_regenerations). Rename the RegenerationSelector protocol to RegenerationPlanner and the dispatcher attribute to `planner`. Co-Authored-By: Claude Opus 4.8 (1M context) --- backend/infrahub/core/branch/tasks.py | 2 +- .../core/merge/regeneration_dispatcher.py | 14 ++-- .../merge/selective_regen/orchestrator.py | 4 +- .../test_merge_selective_regen.py | 2 +- .../merge/test_regeneration_dispatcher.py | 82 +++++++++---------- 5 files changed, 52 insertions(+), 52 deletions(-) diff --git a/backend/infrahub/core/branch/tasks.py b/backend/infrahub/core/branch/tasks.py index 3056de4c83..2ecf8452c3 100644 --- a/backend/infrahub/core/branch/tasks.py +++ b/backend/infrahub/core/branch/tasks.py @@ -503,7 +503,7 @@ async def _build_post_merge_regeneration_dispatcher( ) return PostMergeRegenerationDispatcher( workflow=get_workflow(), - selector=build_merge_selective_regeneration(client=get_client(), log=log, output_capturer=output_capturer), + planner=build_merge_selective_regeneration(client=get_client(), log=log, output_capturer=output_capturer), summary_cache=DiffSummaryCache( cache=await get_cache(), serializer=DiffSummarySerializer(), key_namespace="branch_merge" ), diff --git a/backend/infrahub/core/merge/regeneration_dispatcher.py b/backend/infrahub/core/merge/regeneration_dispatcher.py index 16b989fa0e..29a510da14 100644 --- a/backend/infrahub/core/merge/regeneration_dispatcher.py +++ b/backend/infrahub/core/merge/regeneration_dispatcher.py @@ -24,7 +24,7 @@ from infrahub.services.adapters.workflow import InfrahubWorkflow from .selective_regen.models import PlannedRegeneration, SelectiveRegenerationPlan - from .selective_regen.orchestrator import RegenerationSelector + from .selective_regen.orchestrator import RegenerationPlanner class FullRegenerationReason(StrEnum): @@ -60,12 +60,12 @@ class PostMergeRegenerationDispatcher: def __init__( self, workflow: InfrahubWorkflow, - selector: RegenerationSelector, + planner: RegenerationPlanner, summary_cache: DiffSummaryCache, log: Logger | LoggerAdapter[Logger], ) -> None: self.workflow = workflow - self.selector = selector + self.planner = planner self.summary_cache = summary_cache self.log = log @@ -103,7 +103,7 @@ async def dispatch( # leaving the merge under-regenerated. A single generator run failing is handled granularly in # _dispatch_plan and does not reach here. try: - plan = await self.selector.build_plan(diff_summary=diff_summary, target_branch=target_branch) + plan = await self.planner.build_plan(diff_summary=diff_summary, target_branch=target_branch) await self._dispatch_plan(context=context, target_branch=target_branch, plan=plan) except Exception: self.log.exception("Selective post-merge regeneration failed; falling back to full regeneration") @@ -163,7 +163,7 @@ async def _dispatch_plan( async def _submit(self, *, context: InfrahubContext, entries: list[PlannedRegeneration]) -> None: """Submit each fire-and-forget request, letting the owning selector consolidate its own kind.""" - for entry in self.selector.consolidate_submissions(entries): + for entry in self.planner.consolidate_submissions(entries): for request in entry.requests: await self.workflow.submit_workflow( workflow=entry.workflow, context=context, parameters={"model": request} @@ -188,7 +188,7 @@ async def _reselect_from_cascade_output( for entry in sources: if entry.output is not None: captured.extend(await entry.output.capture(since=since)) - targeted = await self.selector.reselect_from_cascade_output( + targeted = await self.planner.reselect_from_cascade_output( diff_summary=captured, target_branch=target_branch ) except Exception: @@ -206,7 +206,7 @@ async def _full_regeneration( await submit_full_regeneration(workflow=self.workflow, context=context, target_branch=target_branch) async def _submit_full_terminal_regeneration(self, context: InfrahubContext, target_branch: str) -> None: - for regeneration in self.selector.terminal_full_regenerations(target_branch): + for regeneration in self.planner.terminal_full_regenerations(target_branch): await self.workflow.submit_workflow( workflow=regeneration.workflow, context=context, parameters=regeneration.parameters ) diff --git a/backend/infrahub/core/merge/selective_regen/orchestrator.py b/backend/infrahub/core/merge/selective_regen/orchestrator.py index 6b5fc6a605..2b3528c690 100644 --- a/backend/infrahub/core/merge/selective_regen/orchestrator.py +++ b/backend/infrahub/core/merge/selective_regen/orchestrator.py @@ -25,7 +25,7 @@ from .participant import CascadeParticipant -class RegenerationSelector(Protocol): +class RegenerationPlanner(Protocol): """Computes which definitions and members a merge diff requires be regenerated.""" async def build_plan(self, diff_summary: list[NodeDiff], target_branch: str) -> SelectiveRegenerationPlan: @@ -47,7 +47,7 @@ def terminal_full_regenerations(self, target_branch: str) -> list[FullRegenerati ... -class MergeSelectiveRegeneration(RegenerationSelector): +class MergeSelectiveRegeneration(RegenerationPlanner): """Select the definitions a merge changed, narrowed to affected members, across every participant. Runs each injected participant's selector over a single computation of the diff's modified kinds and diff --git a/backend/tests/component/proposed_change/test_merge_selective_regen.py b/backend/tests/component/proposed_change/test_merge_selective_regen.py index 6fff4607f7..b702666430 100644 --- a/backend/tests/component/proposed_change/test_merge_selective_regen.py +++ b/backend/tests/component/proposed_change/test_merge_selective_regen.py @@ -262,7 +262,7 @@ async def test_relevant_kind_change_selects_matching_definitions( ) dispatcher = PostMergeRegenerationDispatcher( workflow=workflow_recorder, - selector=build_merge_selective_regeneration( + planner=build_merge_selective_regeneration( client=client, log=logging.getLogger("test"), output_capturer=generator_output ), summary_cache=DiffSummaryCache( diff --git a/backend/tests/unit/core/merge/test_regeneration_dispatcher.py b/backend/tests/unit/core/merge/test_regeneration_dispatcher.py index 3f2efa1d2c..7193acbf34 100644 --- a/backend/tests/unit/core/merge/test_regeneration_dispatcher.py +++ b/backend/tests/unit/core/merge/test_regeneration_dispatcher.py @@ -52,7 +52,7 @@ def _plan( artifact_generates: list[RequestArtifactDefinitionGenerate] | None = None, source_output: _FakeSourceOutput | None = None, ) -> SelectiveRegenerationPlan: - """Build a plan the way the orchestrator does: one entry per selector, tagged by cascade role.""" + """Build a plan the way the orchestrator does: one entry per planner, tagged by cascade role.""" return SelectiveRegenerationPlan( entries=[ PlannedRegeneration( @@ -76,8 +76,8 @@ def _submitted_entry(requests: list[RequestArtifactDefinitionGenerate]) -> Plann ) -class _FakeSelector: - """A RegenerationSelector that returns a canned plan or raises, recording its invocations.""" +class _FakePlanner: + """A RegenerationPlanner that returns a canned plan or raises, recording its invocations.""" def __init__( self, @@ -238,13 +238,13 @@ def _plan_with_only_artifacts() -> SelectiveRegenerationPlan: def _dispatcher( - selector: _FakeSelector, + planner: _FakePlanner, cache: DiffSummaryCache, recorder: WorkflowRecorder, ) -> PostMergeRegenerationDispatcher: return PostMergeRegenerationDispatcher( workflow=recorder, - selector=selector, + planner=planner, summary_cache=cache, log=logging.getLogger("test"), ) @@ -275,15 +275,15 @@ def _full_regen_submitted(recorder: WorkflowRecorder) -> bool: async def test_flag_off_submits_full_regeneration(disable_selective: None) -> None: recorder = WorkflowRecorder() - selector = _FakeSelector(plan=_plan_with_one_of_each()) + planner = _FakePlanner(plan=_plan_with_one_of_each()) cache = _summary_cache(MemoryCache()) await cache.set(diff_id=DIFF_ID, diff_summary=[]) - await _dispatcher(selector, cache, recorder).dispatch( + await _dispatcher(planner, cache, recorder).dispatch( context=_context(), target_branch=TARGET_BRANCH, merge_diff_cache_key=DIFF_ID ) - # Flag off reproduces the prior blanket path exactly, without consulting the selector. + # Flag off reproduces the prior blanket path exactly, without consulting the planner. assert _full_regen_submitted(recorder) assert recorder.get_submit_calls_for(TRIGGER_ARTIFACT_DEFINITION_GENERATE)[0]["parameters"] == { "branch": TARGET_BRANCH @@ -294,58 +294,58 @@ async def test_flag_off_submits_full_regeneration(disable_selective: None) -> No } assert recorder.get_submit_calls_for(REQUEST_ARTIFACT_DEFINITION_GENERATE) == [] assert recorder.get_submit_calls_for(REQUEST_GENERATOR_DEFINITION_RUN) == [] - assert selector.calls == 0 + assert planner.calls == 0 async def test_missing_key_submits_full_regeneration() -> None: recorder = WorkflowRecorder() - selector = _FakeSelector(plan=_plan_with_one_of_each()) + planner = _FakePlanner(plan=_plan_with_one_of_each()) cache = _summary_cache(MemoryCache()) - await _dispatcher(selector, cache, recorder).dispatch( + await _dispatcher(planner, cache, recorder).dispatch( context=_context(), target_branch=TARGET_BRANCH, merge_diff_cache_key=None ) assert _full_regen_submitted(recorder) - assert selector.calls == 0 + assert planner.calls == 0 async def test_cache_miss_submits_full_regeneration() -> None: recorder = WorkflowRecorder() - selector = _FakeSelector(plan=_plan_with_one_of_each()) + planner = _FakePlanner(plan=_plan_with_one_of_each()) cache = _summary_cache(MemoryCache()) # never seeded - await _dispatcher(selector, cache, recorder).dispatch( + await _dispatcher(planner, cache, recorder).dispatch( context=_context(), target_branch=TARGET_BRANCH, merge_diff_cache_key=DIFF_ID ) assert _full_regen_submitted(recorder) - assert selector.calls == 0 + assert planner.calls == 0 async def test_malformed_summary_submits_full_regeneration() -> None: recorder = WorkflowRecorder() - selector = _FakeSelector(plan=_plan_with_one_of_each()) + planner = _FakePlanner(plan=_plan_with_one_of_each()) memory = MemoryCache() memory.storage[f"branch_merge:diff_id:{DIFF_ID}:diff_summary"] = "{not-valid-json" cache = _summary_cache(memory) - await _dispatcher(selector, cache, recorder).dispatch( + await _dispatcher(planner, cache, recorder).dispatch( context=_context(), target_branch=TARGET_BRANCH, merge_diff_cache_key=DIFF_ID ) assert _full_regen_submitted(recorder) - assert selector.calls == 0 + assert planner.calls == 0 assert recorder.get_submit_calls_for(REQUEST_ARTIFACT_DEFINITION_GENERATE) == [] async def test_empty_plan_dispatches_nothing() -> None: recorder = WorkflowRecorder() - selector = _FakeSelector(plan=_plan()) + planner = _FakePlanner(plan=_plan()) cache = _summary_cache(MemoryCache()) await cache.set(diff_id=DIFF_ID, diff_summary=[]) - await _dispatcher(selector, cache, recorder).dispatch( + await _dispatcher(planner, cache, recorder).dispatch( context=_context(), target_branch=TARGET_BRANCH, merge_diff_cache_key=DIFF_ID ) @@ -354,15 +354,15 @@ async def test_empty_plan_dispatches_nothing() -> None: async def test_selection_failure_falls_back_to_full_regeneration() -> None: recorder = WorkflowRecorder() - selector = _FakeSelector(error=RuntimeError("boom")) + planner = _FakePlanner(error=RuntimeError("boom")) cache = _summary_cache(MemoryCache()) await cache.set(diff_id=DIFF_ID, diff_summary=[]) - await _dispatcher(selector, cache, recorder).dispatch( + await _dispatcher(planner, cache, recorder).dispatch( context=_context(), target_branch=TARGET_BRANCH, merge_diff_cache_key=DIFF_ID ) - assert selector.calls == 1 + assert planner.calls == 1 assert _full_regen_submitted(recorder) assert recorder.get_submit_calls_for(REQUEST_ARTIFACT_DEFINITION_GENERATE) == [] @@ -376,13 +376,13 @@ def _targeted_artifact() -> RequestArtifactDefinitionGenerate: async def test_merge_targets_artifacts_from_generator_output() -> None: recorder = WorkflowRecorder() source_output = _FakeSourceOutput(diff_summary=[{"kind": "TestDevice"}]) - selector = _FakeSelector( + planner = _FakePlanner( plan=_plan_with_one_of_each(source_output=source_output), artifact_plan=[_targeted_artifact()] ) cache = _summary_cache(MemoryCache()) await cache.set(diff_id=DIFF_ID, diff_summary=[]) - await _dispatcher(selector, cache, recorder).dispatch( + await _dispatcher(planner, cache, recorder).dispatch( context=_context(), target_branch=TARGET_BRANCH, merge_diff_cache_key=DIFF_ID ) @@ -392,7 +392,7 @@ async def test_merge_targets_artifacts_from_generator_output() -> None: assert [call["workflow"] for call in recorder.execute_calls] == [REQUEST_GENERATOR_DEFINITION_RUN] assert recorder.get_submit_calls_for(REQUEST_GENERATOR_DEFINITION_RUN) == [] assert source_output.calls == 1 - assert selector.reselect_diffs == [[{"kind": "TestDevice"}]] + assert planner.reselect_diffs == [[{"kind": "TestDevice"}]] submitted = [ call["parameters"]["model"].artifact_definition_name for call in recorder.get_submit_calls_for(REQUEST_ARTIFACT_DEFINITION_GENERATE) @@ -404,11 +404,11 @@ async def test_merge_targets_artifacts_from_generator_output() -> None: async def test_awaits_every_generator_before_capturing_output() -> None: recorder = WorkflowRecorder() source_output = _FakeSourceOutput(diff_summary=[{"kind": "TestDevice"}]) - selector = _FakeSelector(plan=_plan_with_two_generators(source_output=source_output)) + planner = _FakePlanner(plan=_plan_with_two_generators(source_output=source_output)) cache = _summary_cache(MemoryCache()) await cache.set(diff_id=DIFF_ID, diff_summary=[]) - await _dispatcher(selector, cache, recorder).dispatch( + await _dispatcher(planner, cache, recorder).dispatch( context=_context(), target_branch=TARGET_BRANCH, merge_diff_cache_key=DIFF_ID ) @@ -424,16 +424,16 @@ async def test_awaits_every_generator_before_capturing_output() -> None: async def test_merge_without_generator_keeps_selective_artifacts() -> None: recorder = WorkflowRecorder() - selector = _FakeSelector(plan=_plan_with_only_artifacts()) + planner = _FakePlanner(plan=_plan_with_only_artifacts()) cache = _summary_cache(MemoryCache()) await cache.set(diff_id=DIFF_ID, diff_summary=[]) - await _dispatcher(selector, cache, recorder).dispatch( + await _dispatcher(planner, cache, recorder).dispatch( context=_context(), target_branch=TARGET_BRANCH, merge_diff_cache_key=DIFF_ID ) # No source ran, so no output is captured and the artifact selection stays narrow. - assert selector.reselect_diffs == [] + assert planner.reselect_diffs == [] assert len(recorder.get_submit_calls_for(REQUEST_ARTIFACT_DEFINITION_GENERATE)) == 1 assert recorder.get_submit_calls_for(TRIGGER_ARTIFACT_DEFINITION_GENERATE) == [] assert recorder.execute_calls == [] @@ -442,11 +442,11 @@ async def test_merge_without_generator_keeps_selective_artifacts() -> None: async def test_generator_output_capture_failure_falls_back_to_blanket_artifacts() -> None: recorder = WorkflowRecorder() source_output = _FakeSourceOutput(error=RuntimeError("capture boom")) - selector = _FakeSelector(plan=_plan_with_one_of_each(source_output=source_output)) + planner = _FakePlanner(plan=_plan_with_one_of_each(source_output=source_output)) cache = _summary_cache(MemoryCache()) await cache.set(diff_id=DIFF_ID, diff_summary=[]) - await _dispatcher(selector, cache, recorder).dispatch( + await _dispatcher(planner, cache, recorder).dispatch( context=_context(), target_branch=TARGET_BRANCH, merge_diff_cache_key=DIFF_ID ) @@ -461,11 +461,11 @@ async def test_generator_output_capture_failure_falls_back_to_blanket_artifacts( async def test_generator_run_failure_is_isolated_and_regenerates_artifacts_not_generators() -> None: recorder = _FailingGeneratorRecorder(fail_definition="gd1") source_output = _FakeSourceOutput(diff_summary=[{"kind": "TestDevice"}]) - selector = _FakeSelector(plan=_plan_with_two_generators(source_output=source_output)) + planner = _FakePlanner(plan=_plan_with_two_generators(source_output=source_output)) cache = _summary_cache(MemoryCache()) await cache.set(diff_id=DIFF_ID, diff_summary=[]) - await _dispatcher(selector, cache, recorder).dispatch( + await _dispatcher(planner, cache, recorder).dispatch( context=_context(), target_branch=TARGET_BRANCH, merge_diff_cache_key=DIFF_ID ) @@ -484,11 +484,11 @@ async def test_generator_run_failure_is_isolated_and_regenerates_artifacts_not_g assert source_output.calls == 0 -async def test_merge_submits_what_the_selector_consolidates() -> None: - """The dispatcher submits exactly the entries the selector's consolidation returns, via their workflow. +async def test_merge_submits_what_the_planner_consolidates() -> None: + """The dispatcher submits exactly the entries the planner's consolidation returns, via their workflow. - Consolidating the requests (deduping a definition selected by more than one diff) is the selector's - job, unit-tested on the selector; here the dispatcher must submit that result verbatim. + Consolidating the requests (deduping a definition selected by more than one diff) is the planner's + job, unit-tested on the planner; here the dispatcher must submit that result verbatim. """ recorder = WorkflowRecorder() consolidated = [ @@ -504,11 +504,11 @@ async def test_merge_submits_what_the_selector_consolidates() -> None: ) ] source_output = _FakeSourceOutput(diff_summary=[{"kind": "TestDevice"}]) - selector = _FakeSelector(plan=_plan_with_one_of_each(source_output=source_output), submissions=consolidated) + planner = _FakePlanner(plan=_plan_with_one_of_each(source_output=source_output), submissions=consolidated) cache = _summary_cache(MemoryCache()) await cache.set(diff_id=DIFF_ID, diff_summary=[]) - await _dispatcher(selector, cache, recorder).dispatch( + await _dispatcher(planner, cache, recorder).dispatch( context=_context(), target_branch=TARGET_BRANCH, merge_diff_cache_key=DIFF_ID ) From 4b7746b3319a7539df539e02a8f7ad56bfb38024 Mon Sep 17 00:00:00 2001 From: Pol Michel Date: Thu, 30 Jul 2026 09:44:14 +0200 Subject: [PATCH 15/16] refactor(regeneration): build the cascade output once at the composition root Address review feedback: GeneratorTrackingOutput was built per-plan (through a factory) only to bind the selected generators' definition names. Have CascadeSourceOutput.capture take the source's requests instead, so the names are read at capture time and the output holds only its capturer. GeneratorTrackingOutput is now built once in _build_post_merge_regeneration_dispatcher and injected through the planner, and the GeneratorOutputFactory / CascadeSourceOutputFactory layer is removed. Co-Authored-By: Claude Opus 4.8 (1M context) --- backend/infrahub/core/branch/tasks.py | 8 +++-- .../core/merge/regeneration_dispatcher.py | 2 +- .../merge/selective_regen/generator_output.py | 31 +++++-------------- .../core/merge/selective_regen/models.py | 23 +++++--------- .../merge/selective_regen/orchestrator.py | 12 +++---- .../core/merge/selective_regen/participant.py | 18 +++++------ .../test_merge_selective_regen.py | 9 ++++-- backend/tests/helpers/selective_regen.py | 13 +------- .../selective_regen/test_generator_output.py | 21 +++---------- .../selective_regen/test_orchestrator.py | 12 +++---- .../merge/selective_regen/test_participant.py | 28 +++-------------- .../merge/test_regeneration_dispatcher.py | 2 +- 12 files changed, 57 insertions(+), 122 deletions(-) diff --git a/backend/infrahub/core/branch/tasks.py b/backend/infrahub/core/branch/tasks.py index 2ecf8452c3..cc362cc90a 100644 --- a/backend/infrahub/core/branch/tasks.py +++ b/backend/infrahub/core/branch/tasks.py @@ -33,7 +33,10 @@ ) from infrahub.core.merge.regeneration_dispatcher import PostMergeRegenerationDispatcher, submit_full_regeneration from infrahub.core.merge.schema_analyzer import MergeSchemaAnalyzer -from infrahub.core.merge.selective_regen.generator_output import GeneratorTrackingGroupDiffCapturer +from infrahub.core.merge.selective_regen.generator_output import ( + GeneratorTrackingGroupDiffCapturer, + GeneratorTrackingOutput, +) from infrahub.core.merge.selective_regen.orchestrator import build_merge_selective_regeneration from infrahub.core.merge.write_blocker import MergeWriteBlocker from infrahub.core.migrations.exceptions import MigrationFailureError @@ -501,9 +504,10 @@ async def _build_post_merge_regeneration_dispatcher( client=get_client(), branch=branch, ) + generator_output = GeneratorTrackingOutput(capturer=output_capturer) return PostMergeRegenerationDispatcher( workflow=get_workflow(), - planner=build_merge_selective_regeneration(client=get_client(), log=log, output_capturer=output_capturer), + planner=build_merge_selective_regeneration(client=get_client(), log=log, generator_output=generator_output), summary_cache=DiffSummaryCache( cache=await get_cache(), serializer=DiffSummarySerializer(), key_namespace="branch_merge" ), diff --git a/backend/infrahub/core/merge/regeneration_dispatcher.py b/backend/infrahub/core/merge/regeneration_dispatcher.py index 29a510da14..67681250a4 100644 --- a/backend/infrahub/core/merge/regeneration_dispatcher.py +++ b/backend/infrahub/core/merge/regeneration_dispatcher.py @@ -187,7 +187,7 @@ async def _reselect_from_cascade_output( captured: list[NodeDiff] = [] for entry in sources: if entry.output is not None: - captured.extend(await entry.output.capture(since=since)) + captured.extend(await entry.output.capture(since=since, requests=entry.requests)) targeted = await self.planner.reselect_from_cascade_output( diff_summary=captured, target_branch=target_branch ) diff --git a/backend/infrahub/core/merge/selective_regen/generator_output.py b/backend/infrahub/core/merge/selective_regen/generator_output.py index 0d309687d0..ce33e3f473 100644 --- a/backend/infrahub/core/merge/selective_regen/generator_output.py +++ b/backend/infrahub/core/merge/selective_regen/generator_output.py @@ -10,7 +10,7 @@ from infrahub.core.timestamp import Timestamp from infrahub.generators.models import RequestGeneratorDefinitionRun -from .models import CascadeSourceOutput, CascadeSourceOutputFactory +from .models import CascadeSourceOutput if TYPE_CHECKING: from collections.abc import Sequence @@ -42,36 +42,19 @@ class GeneratorMutationDiffCapturer(Protocol): async def capture(self, *, since: Timestamp, generator_definition_names: list[str]) -> list[NodeDiff]: ... -class GeneratorTrackingOutput(CascadeSourceOutput): +class GeneratorTrackingOutput(CascadeSourceOutput[RequestGeneratorDefinitionRun]): """The cascade output of a set of generators, captured through their per-member tracking groups. - Binds the generators' definition names to the capturer so the follow-up can capture their output - without handling the names itself. - """ - - def __init__(self, *, capturer: GeneratorMutationDiffCapturer, definition_names: list[str]) -> None: - self._capturer = capturer - self._definition_names = definition_names - - async def capture(self, *, since: Timestamp) -> list[NodeDiff]: - return await self._capturer.capture(since=since, generator_definition_names=self._definition_names) - - -class GeneratorOutputFactory(CascadeSourceOutputFactory[RequestGeneratorDefinitionRun]): - """Produces a set of generators' cascade output from the runs selected for them. - - Owns the generator-specific step of reading the definition names off the runs, so the capturer it - holds stays free of that kind's request shape. + Holds only the capturer; the runs to capture arrive per call, so it reads their definition names + itself and the follow-up need not handle that kind-specific step. """ def __init__(self, capturer: GeneratorMutationDiffCapturer) -> None: self._capturer = capturer - def for_requests(self, requests: Sequence[RequestGeneratorDefinitionRun]) -> CascadeSourceOutput: - return GeneratorTrackingOutput( - capturer=self._capturer, - definition_names=[run.generator_definition.definition_name for run in requests], - ) + async def capture(self, *, since: Timestamp, requests: Sequence[RequestGeneratorDefinitionRun]) -> list[NodeDiff]: + definition_names = [run.generator_definition.definition_name for run in requests] + return await self._capturer.capture(since=since, generator_definition_names=definition_names) class GeneratorTrackingGroupDiffCapturer(GeneratorMutationDiffCapturer): diff --git a/backend/infrahub/core/merge/selective_regen/models.py b/backend/infrahub/core/merge/selective_regen/models.py index 1428138d1b..e5f3b6feb7 100644 --- a/backend/infrahub/core/merge/selective_regen/models.py +++ b/backend/infrahub/core/merge/selective_regen/models.py @@ -35,24 +35,15 @@ class CascadeRole(Enum): TERMINAL = "terminal" -class CascadeSourceOutput(Protocol): - """Captures the diff of what a cascade source wrote once it has run. +class CascadeSourceOutput[RequestT](Protocol): + """Captures the diff of what a cascade source's requests wrote once they have run. - A source produces the diff of its own writes so the terminals that read them can be reselected, - without the follow-up needing to know how that output is located. + Given the requests the source selected, it produces the diff of their own writes so the terminals + that read them can be reselected, without the follow-up needing to know how that output is located. + Bound to its capturer at wiring time; the requests scope the capture and arrive per run. """ - async def capture(self, *, since: Timestamp) -> list[NodeDiff]: ... - - -class CascadeSourceOutputFactory[RequestT](Protocol): - """Produces the output capture for a cascade source from the requests it selected. - - Bound to a source at wiring time and given that source's selected requests, so the capture is - scoped to what those requests will write without the source itself owning how that is located. - """ - - def for_requests(self, requests: Sequence[RequestT]) -> CascadeSourceOutput: ... + async def capture(self, *, since: Timestamp, requests: Sequence[RequestT]) -> list[NodeDiff]: ... @dataclass(frozen=True) @@ -70,7 +61,7 @@ class PlannedRegeneration: workflow: WorkflowDefinition cascade_role: CascadeRole requests: Sequence[RegenerationRequest] - output: CascadeSourceOutput | None = None + output: CascadeSourceOutput[Any] | None = None """How to capture this entry's output when it is a cascade source; None for a terminal.""" diff --git a/backend/infrahub/core/merge/selective_regen/orchestrator.py b/backend/infrahub/core/merge/selective_regen/orchestrator.py index 2b3528c690..72eead893b 100644 --- a/backend/infrahub/core/merge/selective_regen/orchestrator.py +++ b/backend/infrahub/core/merge/selective_regen/orchestrator.py @@ -8,7 +8,6 @@ from .definition_selector.generator_selector import GeneratorSelector from .fallbacks import repositories_forcing_full_regeneration from .gate import DefinitionGate -from .generator_output import GeneratorOutputFactory from .impacted import ImpactedSubscriberResolver from .models import CascadeRole, PlannedRegeneration, SelectiveRegenerationPlan from .participant import CascadeSource, CascadeTerminal @@ -20,8 +19,7 @@ from infrahub_sdk.client import InfrahubClient from infrahub_sdk.diff import NodeDiff - from .generator_output import GeneratorMutationDiffCapturer - from .models import DefinitionModel, FullRegeneration, RegenerationRequest + from .models import CascadeSourceOutput, DefinitionModel, FullRegeneration, RegenerationRequest from .participant import CascadeParticipant @@ -131,13 +129,13 @@ def build_merge_selective_regeneration( *, client: InfrahubClient, log: logging.Logger | logging.LoggerAdapter[logging.Logger], - output_capturer: GeneratorMutationDiffCapturer, + generator_output: CascadeSourceOutput[Any], ) -> MergeSelectiveRegeneration: """Wire the participants for one merge follow-up, sharing the gate and impact resolver. The generator participant runs before the artifact participant so the plan awaits generator output - before the artifacts that may read it are selected. The generator participant is the cascade source - and carries the output capture built from the capturer, so the follow-up need not own that. + before the artifacts that may read it are selected. The generator participant is the cascade source; + its output capture is built once at the composition root and injected here. """ gate = DefinitionGate(log=log) impacted_resolver = ImpactedSubscriberResolver(client=client) @@ -145,7 +143,7 @@ def build_merge_selective_regeneration( participants=[ CascadeSource( GeneratorSelector(client=client, gate=gate, impacted_resolver=impacted_resolver, log=log), - output=GeneratorOutputFactory(capturer=output_capturer), + output=generator_output, ), CascadeTerminal(ArtifactSelector(client=client, gate=gate, impacted_resolver=impacted_resolver, log=log)), ] diff --git a/backend/infrahub/core/merge/selective_regen/participant.py b/backend/infrahub/core/merge/selective_regen/participant.py index b1feafa7b9..f897c85b36 100644 --- a/backend/infrahub/core/merge/selective_regen/participant.py +++ b/backend/infrahub/core/merge/selective_regen/participant.py @@ -14,7 +14,7 @@ from infrahub.workflows.models import WorkflowDefinition from .definition_selector.base import DefinitionSelectorBase - from .models import CascadeSourceOutput, CascadeSourceOutputFactory, LoadedDefinition, RegenerationRequest + from .models import CascadeSourceOutput, LoadedDefinition, RegenerationRequest class CascadeParticipant[RequestT: RegenerationRequest](ABC): @@ -65,7 +65,7 @@ def to_entry(self, requests: Sequence[RequestT]) -> PlannedRegeneration: workflow=self.workflow, cascade_role=self.role, requests=requests, - output=self._capture(requests), + output=self._output(), ) def consolidated_entry(self, requests: Sequence[RequestT]) -> PlannedRegeneration: @@ -80,8 +80,8 @@ def full_regeneration(self, *, target_branch: str) -> FullRegeneration: ) @abstractmethod - def _capture(self, requests: Sequence[RequestT]) -> CascadeSourceOutput | None: - """The output capture for these requests, or None when nothing downstream reselects from it.""" + def _output(self) -> CascadeSourceOutput[RequestT] | None: + """This participant's cascade output, or None when nothing downstream reselects from it.""" class CascadeSource[RequestT: RegenerationRequest](CascadeParticipant[RequestT]): @@ -90,13 +90,13 @@ class CascadeSource[RequestT: RegenerationRequest](CascadeParticipant[RequestT]) role = CascadeRole.SOURCE def __init__( - self, selector: DefinitionSelectorBase[Any, RequestT], *, output: CascadeSourceOutputFactory[RequestT] + self, selector: DefinitionSelectorBase[Any, RequestT], *, output: CascadeSourceOutput[RequestT] ) -> None: super().__init__(selector) - self._output = output + self._source_output = output - def _capture(self, requests: Sequence[RequestT]) -> CascadeSourceOutput: - return self._output.for_requests(requests) + def _output(self) -> CascadeSourceOutput[RequestT]: + return self._source_output class CascadeTerminal[RequestT: RegenerationRequest](CascadeParticipant[RequestT]): @@ -104,5 +104,5 @@ class CascadeTerminal[RequestT: RegenerationRequest](CascadeParticipant[RequestT role = CascadeRole.TERMINAL - def _capture(self, requests: Sequence[RequestT]) -> None: # noqa: ARG002 + def _output(self) -> None: return None diff --git a/backend/tests/component/proposed_change/test_merge_selective_regen.py b/backend/tests/component/proposed_change/test_merge_selective_regen.py index b702666430..ee7c331a2d 100644 --- a/backend/tests/component/proposed_change/test_merge_selective_regen.py +++ b/backend/tests/component/proposed_change/test_merge_selective_regen.py @@ -15,6 +15,7 @@ from infrahub.core.diff.summary_serializer import DiffSummarySerializer from infrahub.core.initialization import create_branch from infrahub.core.merge.regeneration_dispatcher import PostMergeRegenerationDispatcher +from infrahub.core.merge.selective_regen.generator_output import GeneratorTrackingOutput from infrahub.core.merge.selective_regen.orchestrator import build_merge_selective_regeneration from infrahub.core.node import Node from infrahub.core.schema import AttributeSchema, NodeSchema, SchemaRoot @@ -257,13 +258,15 @@ async def test_relevant_kind_change_selects_matching_definitions( diff_id=DIFF_CACHE_KEY, diff_summary=[make_node_diff(dataset["device1_id"], "TestNetworkDevice", default_branch.name, ["name"])], ) - generator_output = _StubGeneratorDiffCapturer( + capturer = _StubGeneratorDiffCapturer( diff_summary=[make_node_diff(dataset["device1_id"], "TestNetworkDevice", default_branch.name, ["name"])] ) dispatcher = PostMergeRegenerationDispatcher( workflow=workflow_recorder, planner=build_merge_selective_regeneration( - client=client, log=logging.getLogger("test"), output_capturer=generator_output + client=client, + log=logging.getLogger("test"), + generator_output=GeneratorTrackingOutput(capturer=capturer), ), summary_cache=DiffSummaryCache( cache=memory_cache, serializer=DiffSummarySerializer(), key_namespace="branch_merge" @@ -284,7 +287,7 @@ async def test_relevant_kind_change_selects_matching_definitions( assert generator_calls[0]["parameters"]["model"].target_members == [] # The generator is awaited (never submitted) and its output is captured to drive the selection. assert workflow_recorder.get_submit_calls_for(REQUEST_GENERATOR_DEFINITION_RUN) == [] - assert generator_output.calls == 1 + assert capturer.calls == 1 # The affected artifact is regenerated by a selective request, not the blanket trigger. artifact_names = { call["parameters"]["model"].artifact_definition_name diff --git a/backend/tests/helpers/selective_regen.py b/backend/tests/helpers/selective_regen.py index 06771a38d9..166339e577 100644 --- a/backend/tests/helpers/selective_regen.py +++ b/backend/tests/helpers/selective_regen.py @@ -27,27 +27,16 @@ from infrahub_sdk.diff import NodeDiff - from infrahub.core.merge.selective_regen.models import CascadeSourceOutput from infrahub.core.timestamp import Timestamp class StubCascadeSourceOutput: """A CascadeSourceOutput that captures nothing, for source doubles that only exercise selection.""" - async def capture(self, *, since: Timestamp) -> list[NodeDiff]: + async def capture(self, *, since: Timestamp, requests: Sequence[object]) -> list[NodeDiff]: return [] -class StubOutputFactory: - """A CascadeSourceOutputFactory yielding a fixed capture, for source doubles that only select.""" - - def __init__(self, result: CascadeSourceOutput | None = None) -> None: - self._result = result if result is not None else StubCascadeSourceOutput() - - def for_requests(self, requests: Sequence[object]) -> CascadeSourceOutput: - return self._result - - class RecordingGeneratorDiffCapturer: """A GeneratorMutationDiffCapturer double: records its calls and returns its diff unchanged.""" diff --git a/backend/tests/unit/core/merge/selective_regen/test_generator_output.py b/backend/tests/unit/core/merge/selective_regen/test_generator_output.py index f2414204fd..40085f6dcb 100644 --- a/backend/tests/unit/core/merge/selective_regen/test_generator_output.py +++ b/backend/tests/unit/core/merge/selective_regen/test_generator_output.py @@ -11,7 +11,6 @@ from infrahub.core.diff.summary_serializer import DiffSummarySerializer from infrahub.core.merge.selective_regen.generator_output import ( CAPTURE_DIFF_NAME_PREFIX, - GeneratorOutputFactory, GeneratorTrackingGroupDiffCapturer, GeneratorTrackingOutput, ) @@ -185,18 +184,6 @@ async def test_capture_marks_the_saved_diff_so_it_can_be_identified_later() -> N assert coordinator.names[0].startswith(CAPTURE_DIFF_NAME_PREFIX) -async def test_generator_tracking_output_forwards_its_names_and_since_to_the_capturer() -> None: - """GeneratorTrackingOutput binds the generator names to the capturer and forwards the since and diff.""" - capturer = RecordingGeneratorDiffCapturer() - output = GeneratorTrackingOutput(capturer=capturer, definition_names=["gen-a", "gen-b"]) - since = Timestamp() - - captured = await output.capture(since=since) - - assert capturer.calls == [(since, ["gen-a", "gen-b"])] - assert captured is capturer.result - - def _generator_run(name: str) -> RequestGeneratorDefinitionRun: return RequestGeneratorDefinitionRun( branch="main", @@ -219,13 +206,13 @@ def _generator_run(name: str) -> RequestGeneratorDefinitionRun: ) -async def test_generator_cascade_output_captures_the_runs_generators_by_name() -> None: - """GeneratorOutputFactory derives the capture scope from each run's generator name.""" +async def test_generator_tracking_output_captures_the_runs_generators_by_name() -> None: + """GeneratorTrackingOutput reads the names off the runs and forwards them with since to the capturer.""" capturer = RecordingGeneratorDiffCapturer() - output = GeneratorOutputFactory(capturer=capturer).for_requests([_generator_run("gen-a"), _generator_run("gen-b")]) + output = GeneratorTrackingOutput(capturer=capturer) since = Timestamp() - captured = await output.capture(since=since) + captured = await output.capture(since=since, requests=[_generator_run("gen-a"), _generator_run("gen-b")]) assert capturer.calls == [(since, ["gen-a", "gen-b"])] assert captured is capturer.result diff --git a/backend/tests/unit/core/merge/selective_regen/test_orchestrator.py b/backend/tests/unit/core/merge/selective_regen/test_orchestrator.py index 4f7a4cd414..c7dd9567b3 100644 --- a/backend/tests/unit/core/merge/selective_regen/test_orchestrator.py +++ b/backend/tests/unit/core/merge/selective_regen/test_orchestrator.py @@ -25,7 +25,6 @@ ArtifactForcingSelector, GeneratorForcingSelector, StubCascadeSourceOutput, - StubOutputFactory, ) if TYPE_CHECKING: @@ -121,7 +120,7 @@ async def test_build_plan_shares_modified_kinds_and_assembles_plan() -> None: plan = await MergeSelectiveRegeneration( participants=[ - CascadeSource(generator_selector, output=StubOutputFactory(result=generator_output)), + CascadeSource(generator_selector, output=generator_output), CascadeTerminal(artifact_selector), ] ).build_plan(diff_summary=diff_summary, target_branch=TARGET_BRANCH) @@ -159,7 +158,7 @@ async def test_reselect_from_cascade_output_excludes_cascade_sources() -> None: entries = await MergeSelectiveRegeneration( participants=[ - CascadeSource(generator_selector, output=StubOutputFactory()), + CascadeSource(generator_selector, output=StubCascadeSourceOutput()), CascadeTerminal(artifact_selector), ] ).reselect_from_cascade_output(diff_summary=diff_summary, target_branch=TARGET_BRANCH) @@ -218,7 +217,7 @@ async def test_consolidate_submissions_routes_each_workflow_to_its_selector() -> result = MergeSelectiveRegeneration( participants=[ - CascadeSource(generator_selector, output=StubOutputFactory()), + CascadeSource(generator_selector, output=StubCascadeSourceOutput()), CascadeTerminal(artifact_selector), ] ).consolidate_submissions(entries) @@ -234,7 +233,8 @@ async def test_consolidate_submissions_routes_each_workflow_to_its_selector() -> def test_terminal_full_regenerations_cover_every_terminal_and_exclude_sources() -> None: """The source-failure fallback regenerates every terminal kind wholesale, never the sources.""" source = CascadeSource[RequestGeneratorDefinitionRun]( - GeneratorForcingSelector(definitions=[], member_ids=[], subscriber_by_member={}), output=StubOutputFactory() + GeneratorForcingSelector(definitions=[], member_ids=[], subscriber_by_member={}), + output=StubCascadeSourceOutput(), ) terminal = CascadeTerminal(ArtifactForcingSelector(definitions=[], member_ids=[], subscriber_by_member={})) @@ -307,7 +307,7 @@ async def test_missing_generator_fingerprint_escalates_a_sibling_artifact_in_the plan = await MergeSelectiveRegeneration( participants=[ - CascadeSource(generator_selector, output=StubOutputFactory()), + CascadeSource(generator_selector, output=StubCascadeSourceOutput()), CascadeTerminal(artifact_selector), ] ).build_plan(diff_summary=[], target_branch=TARGET_BRANCH) diff --git a/backend/tests/unit/core/merge/selective_regen/test_participant.py b/backend/tests/unit/core/merge/selective_regen/test_participant.py index 5769eab3c5..bebe0e69f3 100644 --- a/backend/tests/unit/core/merge/selective_regen/test_participant.py +++ b/backend/tests/unit/core/merge/selective_regen/test_participant.py @@ -1,34 +1,15 @@ from __future__ import annotations -from typing import TYPE_CHECKING - from infrahub.core.merge.selective_regen.models import CascadeRole from infrahub.core.merge.selective_regen.participant import CascadeSource, CascadeTerminal from infrahub.generators.models import RequestGeneratorDefinitionRun from infrahub.workflows.catalogue import REQUEST_ARTIFACT_DEFINITION_GENERATE, REQUEST_GENERATOR_DEFINITION_RUN from tests.helpers.selective_regen import ArtifactForcingSelector, GeneratorForcingSelector, StubCascadeSourceOutput -if TYPE_CHECKING: - from collections.abc import Sequence - - from infrahub.core.merge.selective_regen.models import CascadeSourceOutput, RegenerationRequest - - -class _RecordingOutputFactory: - """A CascadeSourceOutputFactory that records the requests it was asked to build the capture from.""" - - def __init__(self) -> None: - self.seen: list[Sequence[RegenerationRequest]] = [] - self.result = StubCascadeSourceOutput() - - def for_requests(self, requests: Sequence[RegenerationRequest]) -> CascadeSourceOutput: - self.seen.append(requests) - return self.result - -def test_source_entry_forwards_its_requests_and_carries_the_built_output() -> None: - """A source's entry is tagged SOURCE and carries the capture its output builds from those requests.""" - output = _RecordingOutputFactory() +def test_source_entry_carries_its_output() -> None: + """A source's entry is tagged SOURCE and carries the output it was wired with.""" + output = StubCascadeSourceOutput() selector = GeneratorForcingSelector(definitions=[], member_ids=[], subscriber_by_member={}) participant = CascadeSource[RequestGeneratorDefinitionRun](selector, output=output) requests: list[RequestGeneratorDefinitionRun] = [] @@ -39,8 +20,7 @@ def test_source_entry_forwards_its_requests_and_carries_the_built_output() -> No assert entry.workflow is REQUEST_GENERATOR_DEFINITION_RUN assert entry.cascade_role is CascadeRole.SOURCE assert entry.requests is requests - assert output.seen[0] is requests - assert entry.output is output.result + assert entry.output is output def test_terminal_entry_carries_no_output() -> None: diff --git a/backend/tests/unit/core/merge/test_regeneration_dispatcher.py b/backend/tests/unit/core/merge/test_regeneration_dispatcher.py index 7193acbf34..f18d58aad8 100644 --- a/backend/tests/unit/core/merge/test_regeneration_dispatcher.py +++ b/backend/tests/unit/core/merge/test_regeneration_dispatcher.py @@ -121,7 +121,7 @@ def __init__(self, *, diff_summary: list | None = None, error: Exception | None self._error = error self.calls = 0 - async def capture(self, *, since: Timestamp) -> list: + async def capture(self, *, since: Timestamp, requests: Sequence[Any]) -> list: self.calls += 1 if self._error is not None: raise self._error From 800121fd61ddea3b22b593d81357f4255862e1bb Mon Sep 17 00:00:00 2001 From: Pol Michel Date: Thu, 30 Jul 2026 10:29:35 +0200 Subject: [PATCH 16/16] refactor(regeneration): rename GeneratorTrackingOutput to GeneratorCascadeOutput The class is the generator kind's cascade-source output handle: it holds a capturer and, given the selected requests, delegates the capture after deriving the definition names. It does not itself track anything -- the tracking-group mechanism lives in GeneratorTrackingGroupDiffCapturer, which it delegates to -- so "Tracking" described the delegate, not this object. Name it for its role instead. Co-Authored-By: Claude Opus 4.8 (1M context) --- backend/infrahub/core/branch/tasks.py | 4 ++-- .../infrahub/core/merge/selective_regen/generator_output.py | 2 +- .../component/proposed_change/test_merge_selective_regen.py | 4 ++-- .../core/merge/selective_regen/test_generator_output.py | 6 +++--- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/backend/infrahub/core/branch/tasks.py b/backend/infrahub/core/branch/tasks.py index cc362cc90a..2d07854f48 100644 --- a/backend/infrahub/core/branch/tasks.py +++ b/backend/infrahub/core/branch/tasks.py @@ -34,8 +34,8 @@ from infrahub.core.merge.regeneration_dispatcher import PostMergeRegenerationDispatcher, submit_full_regeneration from infrahub.core.merge.schema_analyzer import MergeSchemaAnalyzer from infrahub.core.merge.selective_regen.generator_output import ( + GeneratorCascadeOutput, GeneratorTrackingGroupDiffCapturer, - GeneratorTrackingOutput, ) from infrahub.core.merge.selective_regen.orchestrator import build_merge_selective_regeneration from infrahub.core.merge.write_blocker import MergeWriteBlocker @@ -504,7 +504,7 @@ async def _build_post_merge_regeneration_dispatcher( client=get_client(), branch=branch, ) - generator_output = GeneratorTrackingOutput(capturer=output_capturer) + generator_output = GeneratorCascadeOutput(capturer=output_capturer) return PostMergeRegenerationDispatcher( workflow=get_workflow(), planner=build_merge_selective_regeneration(client=get_client(), log=log, generator_output=generator_output), diff --git a/backend/infrahub/core/merge/selective_regen/generator_output.py b/backend/infrahub/core/merge/selective_regen/generator_output.py index ce33e3f473..a50e5bcc26 100644 --- a/backend/infrahub/core/merge/selective_regen/generator_output.py +++ b/backend/infrahub/core/merge/selective_regen/generator_output.py @@ -42,7 +42,7 @@ class GeneratorMutationDiffCapturer(Protocol): async def capture(self, *, since: Timestamp, generator_definition_names: list[str]) -> list[NodeDiff]: ... -class GeneratorTrackingOutput(CascadeSourceOutput[RequestGeneratorDefinitionRun]): +class GeneratorCascadeOutput(CascadeSourceOutput[RequestGeneratorDefinitionRun]): """The cascade output of a set of generators, captured through their per-member tracking groups. Holds only the capturer; the runs to capture arrive per call, so it reads their definition names diff --git a/backend/tests/component/proposed_change/test_merge_selective_regen.py b/backend/tests/component/proposed_change/test_merge_selective_regen.py index ee7c331a2d..eba9748aea 100644 --- a/backend/tests/component/proposed_change/test_merge_selective_regen.py +++ b/backend/tests/component/proposed_change/test_merge_selective_regen.py @@ -15,7 +15,7 @@ from infrahub.core.diff.summary_serializer import DiffSummarySerializer from infrahub.core.initialization import create_branch from infrahub.core.merge.regeneration_dispatcher import PostMergeRegenerationDispatcher -from infrahub.core.merge.selective_regen.generator_output import GeneratorTrackingOutput +from infrahub.core.merge.selective_regen.generator_output import GeneratorCascadeOutput from infrahub.core.merge.selective_regen.orchestrator import build_merge_selective_regeneration from infrahub.core.node import Node from infrahub.core.schema import AttributeSchema, NodeSchema, SchemaRoot @@ -266,7 +266,7 @@ async def test_relevant_kind_change_selects_matching_definitions( planner=build_merge_selective_regeneration( client=client, log=logging.getLogger("test"), - generator_output=GeneratorTrackingOutput(capturer=capturer), + generator_output=GeneratorCascadeOutput(capturer=capturer), ), summary_cache=DiffSummaryCache( cache=memory_cache, serializer=DiffSummarySerializer(), key_namespace="branch_merge" diff --git a/backend/tests/unit/core/merge/selective_regen/test_generator_output.py b/backend/tests/unit/core/merge/selective_regen/test_generator_output.py index 40085f6dcb..ea18b38ab2 100644 --- a/backend/tests/unit/core/merge/selective_regen/test_generator_output.py +++ b/backend/tests/unit/core/merge/selective_regen/test_generator_output.py @@ -11,8 +11,8 @@ from infrahub.core.diff.summary_serializer import DiffSummarySerializer from infrahub.core.merge.selective_regen.generator_output import ( CAPTURE_DIFF_NAME_PREFIX, + GeneratorCascadeOutput, GeneratorTrackingGroupDiffCapturer, - GeneratorTrackingOutput, ) from infrahub.core.timestamp import Timestamp from infrahub.generators.models import ProposedChangeGeneratorDefinition, RequestGeneratorDefinitionRun @@ -207,9 +207,9 @@ def _generator_run(name: str) -> RequestGeneratorDefinitionRun: async def test_generator_tracking_output_captures_the_runs_generators_by_name() -> None: - """GeneratorTrackingOutput reads the names off the runs and forwards them with since to the capturer.""" + """GeneratorCascadeOutput reads the names off the runs and forwards them with since to the capturer.""" capturer = RecordingGeneratorDiffCapturer() - output = GeneratorTrackingOutput(capturer=capturer) + output = GeneratorCascadeOutput(capturer=capturer) since = Timestamp() captured = await output.capture(since=since, requests=[_generator_run("gen-a"), _generator_run("gen-b")])