Skip to content

Conversation

@rhuss
Copy link
Collaborator

@rhuss rhuss commented Nov 14, 2025

Note: This PR and the code have been created with spec-kit, and its a test balloon for SDD. Feel free to shortcut the review, but this kind of spec-oriented reviews are at the heart of spec driven development, so its also good to get an initial feeling about that methodology. Feel free to comment also about the process in general below. No bits and bytes have been harmed until now, this here is only the result of the initial specification step. See this nice diagram about a first look on SDD. See the section "About SDD" for another quick intro into the SDD review process.

Overview

This PR implements the specification phase for the External Provider Injection feature, as described in the design document: https://hackmd.io/@7FIgxbJfSliRvRtcYpoXFw/SyswYGr6le/edit

⚠️ Note: This is the first phase containing only specifications - no implementation code is included in this PR.

🎯 Key Architectural Decisions

  1. Two-phase init container architecture: Install (N containers) → Merge (1 container)
  2. CRD-ordered execution: Init containers run in user-specified order (not alphabetical)
  3. extra-providers.yaml schema: Forward-compatible design enabling future migration to native LlamaStack support
  4. Merge tool binary: Included in operator image for Phase 2 init container
  5. Configuration merge order: User ConfigMap run.yaml (if exists) → External providers (with conflict resolution: external providers take precedence over ConfigMap with WARNING logged)

What's in This PR

This PR introduces comprehensive specification documents generated using spec-kit:

📋 Core Specification Documents

  • spec.md ⭐ - Primary review focus: Core requirements, user stories, and acceptance criteria (32 functional requirements)
  • plan.md - Implementation approach with two-phase init container architecture
  • tasks.md - Breakdown of 110 tasks across 7 implementation phases
  • integration-points.md - Detailed integration with existing codebase (10 integration points)
  • extra-providers-schema.md - Forward-compatible schema for future LlamaStack support
  • pr-strategy.md - 8-PR breakdown strategy to manage implementation complexity

📖 Review Roadmap

Recommended Review Order (30-45 minutes total)

Follow this order for an efficient and thorough review:

Roadmap for doing this PR review

Step 1: Quick Context (5 minutes)

  1. Read the Purpose section in spec.md
  2. Skim the User Scenarios & Testing section to understand the 3 user stories

Step 2: Deep Dive - Requirements (15 minutes) ⭐ MOST IMPORTANT

  1. Read spec.md - Functional Requirements

    • Focus on: Do these requirements make sense?
    • Check: Are requirements testable and unambiguous?
    • Look for: Missing requirements or edge cases
  2. Read spec.md - User Story Acceptance Scenarios

    • Validate: Are scenarios realistic and complete?
    • Consider: What scenarios are we missing?

Step 3: Architecture Validation (10 minutes)

  1. Skim integration-points.md - Init Container Architecture

    • Understand: Two-phase flow (Install → Merge)
    • Validate: Does this align with Kubernetes patterns?
  2. Read spec.md - Extra Providers Configuration

    • Check: Does the schema make sense?
    • Consider: Is it forward-compatible?
  3. Read spec.md - Merge Order and Precedence

    • Validate: Does conflict resolution make sense?
    • Check: Are warnings sufficient for override cases?

Step 4: Implementation Planning (5-10 minutes) - OPTIONAL

  1. Skim pr-strategy.md

    • Validate: Is the 8-PR breakdown reasonable?
    • Check: Are PR sizes manageable for review?
  2. Skim tasks.md

    • Understand: Task organization across 7 phases
    • Check: Does sequencing make sense?

Step 5: Edge Cases & Error Handling (5 minutes)

  1. Read spec.md - Edge Cases

    • Validate: Are these the right edge cases?
    • Consider: What are we missing?
  2. Read spec.md - Error Message Requirements

    • Check: Are error formats clear and actionable?

🎯 Key Review Questions

As you review, consider:

Requirements (spec.md):

  • Are the 3 user stories clear and independently testable?
  • Are the 32 functional requirements complete and unambiguous?
  • Are error message formats actionable for users?
  • Are edge cases properly identified?
  • Does the merge order and conflict resolution make sense?

Architecture (integration-points.md):

  • Does the two-phase init container approach make sense?
  • Are the 10 integration points in the right places?
  • Is the extra-providers.yaml schema forward-compatible?

Planning (tasks.md, pr-strategy.md):

  • Is the 110-task breakdown reasonable?
  • Is the 8-PR strategy manageable for reviewers?

Process:

  • Is the SDD approach working well?
  • How can we improve the specification review process?

Review Guidance

🔍 What to Focus On

Primary review areas (in order of importance):

  1. spec.md - User stories, functional requirements, and acceptance criteria

    • Are the 3 user stories clear and testable?
    • Are the 32 functional requirements complete?
    • Are error message formats actionable?
    • Does the configuration merge order make sense?
  2. Use cases and edge cases (in spec.md)

    • Do the acceptance scenarios cover the critical paths?
    • Are edge cases properly identified?
  3. integration-points.md - Does the integration strategy make sense?

    • Are the 10 integration points in the right places?
    • Does the two-phase init container architecture align with the existing codebase?

Secondary review areas (optional deep dive):

📚 Understanding the Spec-Kit Structure

The specifications follow a structured format:

  • spec.md: WHAT we're building (requirements, contracts, success criteria)
  • plan.md: HOW we'll build it (technical approach, architecture, examples)
  • tasks.md: WHEN and in what ORDER (task breakdown, phases, dependencies)

About SDD (Specification-Driven Development)

This PR demonstrates the SDD workflow where we:

  1. Specify first: Create detailed specifications before writing code
  2. Review specs: Get alignment on requirements and design (← you are here)
  3. Implement: Build according to validated specifications
  4. Verify: Ensure implementation matches spec

🤔 SDD Review Process

This may be different from typical code reviews:

  • Focus on requirements clarity rather than implementation details
  • Check for ambiguity in specifications
  • Validate assumptions about existing behavior
  • Ensure testability of acceptance criteria
  • Challenge architectural decisions before code is written

We're all learning this process, so feedback on the SDD approach itself is equally welcome!

Questions for Reviewers

  1. Are the user stories (spec.md) clear and complete?
  2. Does the two-phase init container architecture make sense?
  3. Does the configuration merge order and conflict resolution make sense?
  4. Are there integration points we're missing?
  5. Does the extra-providers.yaml schema feel future-proof?
  6. Is the 8-PR breakdown strategy (pr-strategy.md) reasonable?
  7. Process feedback: How can we improve the SDD review workflow?

Next Steps

After this PR is approved:

  1. Begin implementation following the 8-PR strategy (see pr-strategy.md)
  2. Start with first PR: Foundation - CRD Schema & Types (~150 lines)
  3. Each implementation PR will reference back to these specifications

Related Links

@rhuss rhuss force-pushed the 001-deploy-time-providers-l1 branch 4 times, most recently from 6f3ff68 to ea43b14 Compare November 14, 2025 17:41
@rhuss rhuss changed the title Specification: External Provider Injection (Deploy-Time Modularity Level 1) Specification: Deploy-Time Modularity - Level 1 Nov 14, 2025
@rhuss rhuss changed the title Specification: Deploy-Time Modularity - Level 1 Specification: Deploy-Time Modularity - Level 1 (init-container) Nov 14, 2025
@rhuss rhuss force-pushed the 001-deploy-time-providers-l1 branch 3 times, most recently from 3e1a0e5 to d75d7b9 Compare November 15, 2025 16:20
- Challenge patterns that diverge without good reason

---

Copy link
Collaborator

Choose a reason for hiding this comment

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

How does this work for PRs that do not use AI (maybe a small change/update), is there a way to ensure these rules are validated?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That's a good question. The ideal solution would to add this project-level constitution.md to the automated coderabbit review, not sure though which customization hooks are offered by corerabbit.

@VaishnaviHire
Copy link
Collaborator

@mergify rebase

@mergify
Copy link

mergify bot commented Nov 19, 2025

rebase

✅ Branch has been successfully rebased

@VaishnaviHire VaishnaviHire force-pushed the 001-deploy-time-providers-l1 branch from d75d7b9 to 2e8e8cc Compare November 19, 2025 15:32
@VaishnaviHire
Copy link
Collaborator

@mergify rebase

Add comprehensive specification for deploy-time modularity enabling users to
inject custom and third-party provider implementations without rebuilding
distribution images.

Key components:
- Complete specification (spec.md) with 32 functional requirements, 5 NFRs
- Implementation plan (plan.md) with 7 development phases
- Task breakdown (tasks.md) with 110 tasks organized by user story
- Container startup sequence with mermaid diagram showing 5 runtime phases
- Security considerations and forward compatibility planning
- Constitution updates for git commit guidelines and DCO sign-off

Technical approach:
- Init container architecture for provider installation
- Config merge using extra-providers.yaml schema
- Forward compatible with future LlamaStack native support
- Comprehensive error handling and status reporting

Documentation:
- Preflight validation specification (lls-preflight-spec.md)
- Complete CRD structure with validation tags
- Detailed error message formats
- Provider image packaging contract

Constitution updates (v1.4):
- Git commit message conventions
- AI-assisted commit guidelines (Assisted-by trailer format)
- Commit sign-off requirement (DCO)
- Commit hygiene standards

Assisted-by: Claude Code
Signed-off-by: Roland Huß <rhuss@redhat.com>
@mergify
Copy link

mergify bot commented Nov 20, 2025

rebase

✅ Branch has been successfully rebased

@VaishnaviHire VaishnaviHire force-pushed the 001-deploy-time-providers-l1 branch from 2e8e8cc to a8bd2ef Compare November 20, 2025 15:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants