Skip to content

refactor(regeneration): inject selectors so each owns its kind-specific logic - #10055

Open
polmichel wants to merge 6 commits into
developfrom
pmi-20260727-refactor-selectors
Open

refactor(regeneration): inject selectors so each owns its kind-specific logic#10055
polmichel wants to merge 6 commits into
developfrom
pmi-20260727-refactor-selectors

Conversation

@polmichel

@polmichel polmichel commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Summary

Refactors the post-merge selective-regeneration selectors so the merge follow-up no longer reaches
into kind-specific details. Addresses the review feedback that typing the orchestrator against a
specific pair of selectors "defeats the purpose" of the shared DefinitionSelectorBase.

Jira: IFC-2916 (internal, under IFC-2704)
Origin: #9924 (comment)

Change

The orchestrator now holds an injected list of selectors and iterates them generically. Each
selector owns the facts the follow-up used to extract by narrowing:

  • workflow — the workflow that runs its requests.
  • cascade roleSOURCE (produces output the cascade re-reads) or TERMINAL (ends the chain).
  • output capture — a cascade source returns a CascadeSourceOutput bound to the generators it
    selected, so its output is captured without the follow-up handling generator specifics.
  • consolidation — a terminal selector merges requests for the same definition; the follow-up no
    longer knows how to dedup them.

The plan becomes a list of entries (one per selector), each tagged with that metadata. The dispatcher
groups the entries by cascade role — sources run to completion, then each source captures its own
output and the terminals that read it are reselected and consolidated by their owning selector — so
it holds no kind-specific logic (no isinstance on requests, no generator-name extraction, no
artifact consolidation). Adding a definition kind is now one new selector at the wiring layer.

Behavior is unchanged: same selective dispatch, same generator→artifact cascade, same per-generator
failure isolation and safe fallbacks.

How tested

  • Unit: selector metadata and the entries plan; build_plan tags each entry with its output;
    reselect_from_cascade_output excludes the sources; output_capture derives the capture scope
    from a generator's runs; ArtifactSelector.consolidate merges per definition; the orchestrator
    routes consolidation to each owning selector; the dispatcher submits what the selector returns.
  • Component: the real-graph selection flow drives the rebuilt dispatcher end-to-end.

Backend-only refactor; no schema, config, event, CLI, docs, or frontend changes, so no changelog
fragment.

How to review

  1. core/merge/selective_regen/definition_selector/base.py — the per-selector contract
    (workflow, cascade_role, output_capture, consolidate).
  2. core/merge/selective_regen/orchestrator.py — the injected list, build_plan, and
    consolidate_submissions.
  3. core/merge/regeneration_dispatcher.py — role-based dispatch, now free of kind-specific logic.
  4. core/branch/tasks.py — the wiring (the capturer now lives on the generator selector).

🤖 Generated with Claude Code

https://claude.ai/code/session_01B7rfYnzxsPEm45r3e2NnLm

Review in cubic

polmichel and others added 4 commits July 28, 2026 09:58
…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) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01B7rfYnzxsPEm45r3e2NnLm
…rator

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) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01B7rfYnzxsPEm45r3e2NnLm
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) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01B7rfYnzxsPEm45r3e2NnLm
… 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) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01B7rfYnzxsPEm45r3e2NnLm
@github-actions github-actions Bot added the group/backend Issue related to the backend (API Server, Git Agent) label Jul 28, 2026
@codspeed-hq

codspeed-hq Bot commented Jul 28, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 12 untouched benchmarks


Comparing pmi-20260727-refactor-selectors (2eca76f) with develop (fe8fb2c)

Open in CodSpeed

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

All reported issues were addressed across 16 files

Shadow auto-approve: would not auto-approve because issues were found.

Re-trigger cubic

Comment thread backend/infrahub/core/merge/selective_regen/models.py
Comment thread backend/infrahub/core/merge/selective_regen/orchestrator.py Outdated
@polmichel polmichel changed the title refactor(regeneration): inject selectors so each owns its kind-specific logic [IFC-2916] refactor(regeneration): inject selectors so each owns its kind-specific logic Jul 28, 2026
polmichel and others added 2 commits July 28, 2026 10:56
…e 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) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01B7rfYnzxsPEm45r3e2NnLm
…ure 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) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01B7rfYnzxsPEm45r3e2NnLm

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

0 issues found across 3 files (changes from recent commits).

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Shadow auto-approve: would not auto-approve. Auto-approval blocked by 1 unresolved issue from previous reviews.

Re-trigger cubic

@polmichel
polmichel marked this pull request as ready for review July 28, 2026 09:36
@polmichel
polmichel requested a review from a team as a code owner July 28, 2026 09:36
gate: DefinitionGate,
impacted_resolver: ImpactedSubscriberResolver,
log: logging.Logger | logging.LoggerAdapter[logging.Logger],
output_capturer: GeneratorMutationDiffCapturer,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

if possible, I think this should be restructured in a few ways

  1. output_capturer should just be a dependency of GeneratorTrackingOutput. GeneratorSelector does not actually use it, so GeneratorMutationDiffCapturer should not be a dependency of GeneratorSelector
  2. definition_names looks more like an argument to the GeneratorTrackingOutput constructor, but maybe that is not possible. if that's the case, then GeneratorTrackingOutput can have a set_definition_names method and guard on capture() that prevents it from being called with no definition names. or a no-op in that case would be fine too
  3. constructing an instance of CascadeSourceOutput does not look like it is really the responsibility of GeneratorSelector. it looks like you need some way to link this GeneratorSelector to its implementation of CascadeSourceOutput, but I don' think this class should be building the other class

maybe some sort of shared CachedDefinitionLoader class could make some of this work where multiple components share a single definition loader instance


"""
output = selector.output_capture(requests)
if selector.cascade_role is CascadeRole.SOURCE and output is None:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

kind of goes with my above structuring comment, but, ideally, this kind of thing is raised during construction instead of runtime

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

group/backend Issue related to the backend (API Server, Git Agent)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants