You are viewing an old version of this page. View the current version.

Compare with Current View Page History

« Previous Version 12 Next »

Status: Draft

Author: Viquar Khan (Vaquar.khan@gmail.com)

Discussion thread: TBD

Jira:

Motivation

The open-source community is seeing a sharp increase in automated pull requests. While code generation tools can help developers, they frequently produce pull requests that look correct on the surface but contain empty boilerplate, unnecessary comments, and hallucinated logic. This puts a heavy burden on maintainers who spend limited review time on code with no real substance.

While this trend is accelerated by AI, the necessity for structural validation applies to all pull requests. Other major Apache projects are already dealing with this. Apache Airflow changed their contribution policies after automated bots started taking over open issues. Apache Iceberg wrote strict new contribution guidelines to protect their reviewers.

Why AGENTS.md is not enough

Adding an AGENTS.md or CLAUDE.md file is a "soft control." AI models use probabilistic reasoning; if their context window fills up or a user overrides the prompt, the agent will silently ignore markdown instructions and submit bad code anyway. We need a deterministic "hard control" that catches these failures before they reach a human reviewer.

I propose adding two focused validation tasks to Kafka's Gradle build—known collectively as the Automated Integrity Validation (AIV) Gate that catch the two most damaging categories of low-quality contributions: scaffolding-heavy PRs with no real logic, and code that violates Kafka's specific architectural rules. This effectively addresses the "Reviewer Overload" crisis while maintaining Kafka's domain-specific invariants and ensuring 100% local data sovereignty.

Public Interfaces

This KIP introduces no changes to the Kafka protocol, public APIs, client behaviors, or broker metrics. It modifies only the project's internal build tools and CI workflow.

Proposed Changes

I propose adding an AIV Gate using Gradle's includeBuild composite build feature. This relies on native Java classes that live inside the Kafka repository — no external dependencies beyond JavaParser (build-time only, same pattern as Checkstyle/SpotBugs). Developers run them locally with ./gradlew checkContributionQuality, and CI runs them automatically as part of ./gradlew check.

Important: These tasks complement, not duplicate, Kafka's existing checks.

Relationship to Existing Checks

To clarify why these new gates are required alongside existing tooling, the following table details how the proposed AIV Gate differs from Kafka's existing Checkstyle configurations:

Existing CheckWhat it EnforcesWhat the Proposed AIV Gate Does Differently
ImportControlRestricts which packages can import from which globally.The Design Gate enforces conditional architectural patterns within a file (e.g., if X is instantiated, it must be closed).
RegexpGlobally blocks specific string matches (e.g., System.exit()).The Design Gate handles complex AST relationships that regex cannot express safely, such as blocking ExecutorService only if KafkaConsumer is also present.
CyclomaticComplexity / MethodLength / NPathComplexityMeasures the complexity and size of individual methods.The Density Gate (LDR) measures the ratio of executable logic to empty scaffolding across the entire PR diff to catch boilerplate inflation.

Task 1: Logic Density Validation (LDR)

The problem Checkstyle cannot solve: Checkstyle measures the complexity of individual methods. It cannot answer the question: "Did this PR add real logic, or is it 300 lines of scaffolding wrapping 2 lines of actual work?" High-volume, low-effort tools often generate massive PRs with very little executable logic empty class hierarchies, verbose documentation for trivial getters, and copy-pasted boilerplate.

How it works: The task uses JavaParser to build an Abstract Syntax Tree (AST) of each changed Java file in the PR diff and counts two categories of nodes:

  • Logic nodes (weighted): if, for, while, switch (weight 5); method calls, binary expressions (weight 2). Rationale: Control flow nodes receive a higher weight because they introduce actual execution paths, validating human effort.

  • Structure nodes (weighted): class declarations, method declarations (weight 1). Rationale: Structural nodes are merely declarative.

  • Calculation: The tool simply divides the total score of logic nodes by the total score of structural nodes. To ensure accuracy and prevent gaming the system, the AST parser entirely ignores whitespace, comments, and Javadoc.

Default Thresholds & Exceptions:

  • LDR Threshold Calibration (Java): The provisional threshold is 0.25. Interface-heavy PRs (e.g., KIP ConfigDef additions) will be identified via AST and evaluated under a separate, lenient declarative profile. The definitive threshold will be finalized by publishing a sensitivity analysis of the 5th percentile distribution across 100-200 historically merged Kafka PRs during the "Shadow Mode" phase.

  • The "Annotation Attack" Protection: AI models sometimes pad code with complex type annotations to "fake" logic density. The logic parser is tuned to ignore annotations and focus strictly on executable control-flow nodes.

  • Entropy Threshold (Kotlin/Scala/Shell): Because JavaParser does not support Kotlin or Scala, these languages fall back to a simple Shannon entropy analysis to check for repetitive character distributions. However, configuration files (.yaml, .properties, .json, .xml) are inherently repetitive and are completely exempt from these checks. Thresholds for Kotlin and Scala will be established per-extension during Shadow Mode.

  • Refactoring Exception: If a PR has a net-negative line count (e.g., <= -50 lines), the density check is skipped entirely so legitimate cleanups are never blocked.

Task 2: Design Compliance (Kafka Architecture Linter)

The problem Checkstyle cannot solve: While Checkstyle's Regexp module is excellent for globally blocking anti-patterns like System.exit(), it cannot express conditional rules like "if a file uses KafkaConsumer, then it must NOT use ExecutorService."

Governance Risk & Mitigation: To prevent "rule creep," rules are defined in .validation/design-rules.yaml. Adding or removing rules requires a lazy consensus vote on the mailing list. The YAML schema explicitly requires an added-in-version and rationale field.

Kafka-Specific Rules to Ship With:

  • no-direct-zk-access:

    • Trigger: ZooKeeper, ZkClient, CuratorFramework

    • Forbidden: Instantiation or use in new code.

    • Rationale: Kafka removed ZK dependency in KRaft mode, but AI tools still routinely generate ZK-based code from old training data.

  • consumer-thread-safety:

    • Trigger: KafkaConsumer, consumer.poll

    • Forbidden: ExecutorService, ThreadPoolExecutor, newFixedThreadPool

    • Rationale: KafkaConsumer is explicitly not thread-safe. AI tools frequently hallucinate and wrap it in thread pools.

  • producer-close:

    • Trigger: KafkaProducer, new KafkaProducer

    • Required: producer.close

    • Rationale: Unclosed producers leak connections and memory. (Note: The AST check resolves variable references to handle arbitrary names like var p = new KafkaProducer, and successfully detects try-with-resources closures. Spring Kafka and Streams internal producers are dynamically exempted).

  • no-deprecated-metrics:

    • Trigger: MetricName, KafkaMetricsGroup

    • Forbidden: kafka.metrics.KafkaMetricsGroup

    • Rationale: The old metrics API is deprecated; new code must use the updated API.

Implementation Details

Where the code lives: To prevent compilation errors from breaking the entire Gradle build (a common risk with buildSrc/), this tool will be implemented as an isolated composite build using includeBuild("build-plugins").

build-plugins/

├── build.gradle

└── src/

├── main/java/org/apache/kafka/gradle/integrity/

│ ├── DensityAnalyzer.java # LDR + entropy calculation

│ ├── DesignComplianceChecker.java # YAML rule enforcement

│ ├── ContributionQualityTask.java # Gradle task entry point

│ └── DiffParser.java # Reads GitHub PR.patch natively

└── test/java/org/apache/kafka/gradle/integrity/

├── DensityAnalyzerTest.java

└── DesignComplianceCheckerTest.java

Build-Time Dependencies:

  • com.github.javaparser:javaparser-core (AST parsing for density analysis. Build-time only, does not ship in Kafka binaries).

  • org.yaml:snakeyaml (Already a transitive dependency in Kafka's build, used for rule parsing).

Human-in-the-Loop (HITL) Overrides:

  • Secure Emergency Bypass: Adding /aiv skip in any commit message skips all gates, but only if the GitHub Actions actor is on the official COMMITTERS list. Bots and external contributors cannot bypass the gate.

  • Trusted authors: Committers can add their Apache IDs or GitHub usernames (not fragile email addresses) to a trusted_authors list in .validation/config.yaml to bypass density checks entirely.

Strategic Fixes: A Mentorship Framework, not a Binary Gate

To ensure the tool does not bottleneck legitimate contributions, demoralize first-time contributors, or fall victim to AI hallucinations, the system relies on a "Mentorship Framework":

  1. Diagnostic Reports (Fixing the Silence): Instead of a cryptic failure, the tool posts a diagnostic summary in the CI output: "This PR is 70% boilerplate. Try moving logic to the internal Config classes to improve density." This provides a clear, mentorship-oriented path to "ready."

  2. Activity Heartbeat for Assignments: To prevent "Workflow Friction" and "Issue Squatting," we do not strictly block the initial assignment of issues. Instead, we use AIV to re-claim stagnant issues. If an assigned user submits a "hollow" PR that fails the Density Check, the gate flags the PR and frees the issue assignment for others, ensuring project momentum.

  3. BOM-Grounding: The Dependency Gate will actively verify that every method call suggested by the AI actually exists in the current trunk AST. This stops confident-but-wrong "hallucinated" API calls that look like logic but are invalid.

  4. Context Retrieval Guard: Prompts and instructions for AI contributors require them to use "Step-Back Prompting" to first state the intent of the change (from the JIRA) before evaluating the code to ensure the logic isn't "drifting".

Compatibility, Deprecation, and Migration Plan

This proposal does not change the Kafka protocol or public APIs. It adds only internal build logic.

Migration: None. Existing code passes both checks (validated against trunk). The tasks only evaluate changed files in a PR diff, not the entire codebase.

Test Plan

To guarantee zero disruption to developer velocity, this KIP relies on strict unit testing and a phased CI rollout.

1. Unit Tests (JUnit 5 in build-plugins/src/test/):

  • DensityAnalyzerTest:

    • Case: 150 lines of Javadoc with a single return statement -> LDR = 0.05 -> FAILS.

    • Case: 20 lines of branching if/while logic -> LDR = 0.45 -> PASSES.

    • Case: PR removes 100 lines of dead code -> Net LOC negative -> SKIPS check.

  • DesignComplianceCheckerTest:

    • Case: File initializes KafkaConsumer and passes it to Executors.newFixedThreadPool(5) -> FAILS (consumer-thread-safety violation).

    • Case: File initializes KafkaConsumer in a standard single-threaded poll loop -> PASSES.

2. Shadow Mode (30-Day Data Collection):

Upon merge, the GitHub Action will run with continue-on-error: true. It will log results (Pass/Fail, LDR score, AST violations) to the GitHub Actions step summary for 30 days without blocking PRs. During this time, the PMC will publish a sensitivity analysis of the historical data. Once the PMC validates that the false-positive rate is practically zero, we will remove the continue-on-error flag to make it a mandatory, blocking check.

Review & Comparison: Industry Solutions

This solution moves beyond the "descriptive" nature of standard AI tools and introduces a "deterministic" layer of governance.

CapabilityStandard Industry Tools (Copilot/CodeRabbit)Proposed AIV-Gate Solution
Logic FilteringLLM-based summary; flags redundancy probabilistically.LDR (Logic Density Ratio): Deterministic AST-based mathematical gate.
Architectural RulesGeneral best practices (e.g., DRY, SOLID).Design Gate: Enforces Kafka-specific constraints (e.g., ZK blocking, Consumer thread safety).
Supply ChainGeneral CVE scanning (SAST).Dependency Gate: Cross-references imports with project lockfiles locally.
Data PrivacyCloud-based; requires indexing/API keys.100% Local: No data leaves the committer's environment or GitHub Action.

FAQ

Q: Does LDR penalize good documentation?

A: No. The AST analyzer ignores Javadoc and comments entirely, focusing only on executable nodes.

Q: How do I test my PR locally?

A: Run ./gradlew checkContributionQuality to see your LDR score and design violations before you push.

Q: What about Kotlin and Scala code?

A: For Kotlin, Scala, and shell scripts, we use Shannon entropy to detect repetitive, low-substance boilerplate. Pure configuration files (like .yaml or .properties) are completely exempt from these entropy checks.

Q: Is my code being sent to a third party?

A: No. 100% local execution ensures Kafka's IP never leaves our controlled environment.

Q: Won't failing a CI check generate massive email spam to the dev@kafka mailing list?

A: No. The validation task will simply fail the CI check exactly like Checkstyle does today. It will output a clear error message in the console pointing to CONTRIBUTING.md. We will not use automated bot comments to avoid webhook noise.

Q: Checkstyle already handles our code quality. Why do we need a new AST Design Gate?

A: Checkstyle is fantastic for formatting and simple regex (like globally blocking System.exit()), but it struggles with complex, conditional architectural logic. Checkstyle cannot easily enforce "If KafkaConsumer is used, ensure it is not wrapped in java.util.concurrent." This custom task handles the architectural patterns that Checkstyle structurally cannot.

Q: What if the script has a bug and blocks legitimate human contributors?

A: The system prioritizes developer velocity. Any Kafka Committer can bypass the gate immediately by adding /aiv skip to their commit message. The CI runner verifies the actor against the committers list to prevent abuse.

Q: Will parsing the AST significantly slow down local builds (./gradlew check) or CI?

A: No. By isolating the tool via includeBuild and utilizing the native GitHub Actions .patch payload (via DiffParser), there is no shelling out to the git binary. The JavaParser cold-start and AST traversal takes ~2-5 seconds total per PR, not per file.

Q: Who maintains the .validation/design-rules.yaml file? Won't it become a dumping ground for arbitrary rules?

A: The Kafka PMC owns the configuration file. Adding or removing a design rule requires a standard pull request and lazy consensus on the mailing list, and the schema dictates providing a rationale to prevent rule creep.

Q: What happens with trivial PRs, like fixing a single typo in a Javadoc? Won't the LDR score be 0?

A: The Gradle task includes a minimum line-change threshold. Trivial PRs (e.g., modifying fewer than 10 lines of code) bypass the LDR check entirely to ensure minor documentation fixes or typo corrections are never blocked.

Q: Does a "Green" AIV status mean the PR is safe from sophisticated exploits like ShadowRay?

A: No. AIV is a first line of defense against structural slop and known design anti-patterns. It does not replace human reviewers for semantic security. Complex, logic-dense code generated by AI could still contain subtle unauthenticated execution paths. A passing AIV score simply ensures the code has enough substance to warrant human architectural review.

Q: How does the gate handle refactoring?

A: AIV includes a "Refactor Exception"—if a PR removes more lines than it adds (net-negative lines), the density check is automatically bypassed.

Rejected Alternatives

  • Relying solely on human reviewers: Continuing to manually review and close low-substance PRs does not scale and exacerbates maintainer burnout.

  • Restricting PR access to Collaborators Only: While GitHub recently introduced this feature to stop bot spam, using it heavily restricts legitimate, first-time open-source contributors from participating in Kafka.

  • Using Third-Party GitHub Actions: Using pre-compiled external binaries for PR validation introduces supply-chain security risks and prevents developers from running the exact same checks locally on their laptops. Building it natively into our Gradle scripts solves both issues.

  • Adding only AGENTS.md: A soft control that AI models probabilistically ignore. This KIP provides deterministic hard control.

  • No labels