Skip to content

Document dual-model (Application + ComponentGroup) completeness requirement in AGENTS.md #1605

Description

@fullsend-ai-retro

What happened

PR #1543 added distributed tracing with timing span emission across integration-service controllers. The fullsend-ai-review bot ran 10 iterations, gemini-code-assist posted 5 suggestions, and CodeQL flagged one issue. Despite this automated coverage, the most critical bugs were found only by human reviewer kasemAlem on 2026-05-18: two missing emission paths in the Application model flow of buildpipeline_adapter.go. Specifically:

  1. The EnsureSnapshotExists (ComponentGroup flow) had emission but EnsureSnapshotExistsApplication did not.
  2. An early return in EnsureSnapshotExistsApplication when the snapshot annotation already exists skipped timing span emission entirely on restarts/requeues.

These were domain-specific completeness bugs that required knowledge of the dual-model architecture. No automated reviewer flagged them because the AGENTS.md file does not document the dual-model pattern as a completeness requirement.

What could go better

The repo's AGENTS.md documents the adapter pattern, controller structure, and development guidelines, but does not mention that integration-service maintains two parallel flows — ComponentGroup (new) and Application (legacy, planned for deprecation) — and that feature implementations in controllers must cover both. This context gap is the root cause of the automated review miss.

The dual-model pattern is visible in code (e.g., NewAdapter() vs NewAdapterWithApplication() in snapshot_adapter.go, TODO comments about deprecation) but is never stated as a completeness requirement. Human reviewers like kasemAlem know this from experience; automated reviewers need it spelled out.

Confidence: High. The evidence is clear — kasemAlem's comment explicitly says "if you want to continue with implementing this distributed tracing feature you should take care of both flows" and points to two specific locations. This is a repeatable pattern: any future cross-cutting feature (metrics, logging changes, new annotations) faces the same risk of incomplete dual-model coverage.

Proposed change

Add a "Dual-Model Architecture" section to AGENTS.md (which serves as the repo's CLAUDE.md) under the Architecture heading. The section should:

  1. Explain that integration-service currently supports two parallel flows: ComponentGroup model (new) and Application model (legacy, planned for deprecation per README.md).
  2. State explicitly that any feature touching controllers/adapters must be implemented and tested for both flows until the Application model is fully deprecated.
  3. List the key dual-path locations: snapshot_adapter.go (NewAdapter vs NewAdapterWithApplication), buildpipeline_adapter.go (EnsureSnapshotExists vs EnsureSnapshotExistsApplication), and related test files.
  4. Note that TODO comments marked "remove when we deprecate old application model" indicate Application-model code paths that must be maintained.

Example addition after the "Adapter Pattern" subsection:

### Dual-Model Architecture

The service supports two parallel resource flows: **ComponentGroup** (new model) and **Application** (legacy, planned for deprecation). Controllers and adapters implement both flows side-by-side (e.g., `EnsureSnapshotExists` vs `EnsureSnapshotExistsApplication`). Any cross-cutting feature — tracing, metrics, annotations, error handling — must cover both flows until the Application model is fully removed. Look for `TODO: remove when we deprecate old application model` comments to identify Application-model code paths.

Validation criteria

  1. The next cross-cutting feature PR to integration-service (touching 2+ controllers) should have automated review findings that reference dual-model completeness, or should not require a human to catch missing coverage in one of the two flows.
  2. The AGENTS.md file contains an explicit mention of both ComponentGroup and Application flows as a completeness requirement.
  3. Measurable over the next 3 PRs that touch controller adapters: zero human-only catches of missing dual-model coverage that automated reviewers should have flagged.

Generated by retro agent from #1543

Metadata

Metadata

Assignees

No one assigned

    Labels

    documentationImprovements or additions to documentationgood first issueGood for newcomersready-to-codeLets the agent know that issue is ready for fix

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions