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

Compare with Current View Page History

« Previous Version 4 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.

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 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.

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 two new Gradle tasks to Kafka's buildSrc directory. These are 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. Kafka already enforces FinalLocalVariable, CyclomaticComplexity (max 16), MethodLength (max 170), and NPathComplexity (max 500) via checkstyle/checkstyle.xml. The proposed tasks target problems that Checkstyle structurally cannot detect.

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?" AI tools are notorious for generating massive PRs with very little executable logic—empty class hierarchies, verbose documentation for trivial getters, and copy-pasted boilerplate. These PRs pass all existing checks because each individual method is simple.

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:

  1. Logic nodes (weighted): if, for, while, switch (weight 5); method calls, binary expressions (weight 2).

  2. Structure nodes (weighted): class declarations, method declarations (weight 1). Formula: Logic Density Ratio (LDR) = logicNodes / (logicNodes + structureNodes)

Default Thresholds & Exceptions:

  • LDR Threshold (Java): 0.25 (Configurable). We validated this against 50 recent Kafka PRs. Legitimate PRs score 0.3+ because constructors, validation logic, and builder method calls count as logic nodes. Known AI-generated scaffolding PRs score 0.05-0.15.

  • Entropy Threshold (non-Java): 3.8 bits. For Scala, shell scripts, and configs, the task falls back to Shannon entropy. Copy-pasted boilerplate has low character entropy; real code has higher entropy.

  • 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.

  • Test/Generated Code Bypass: Paths matching **/generated/** or **/test/** are automatically excluded to prevent false positives.

Task 2: Design Compliance (Kafka Architecture Linter) The problem Checkstyle cannot solve: Checkstyle's Regexp module can block individual patterns globally (e.g., System.exit). But it cannot express conditional rules like "if a file uses KafkaConsumer, then it must NOT use ExecutorService." It also cannot express required patterns like "if a file creates a KafkaProducer, it should call close()."

How it works: Rules are defined in a simple YAML file (.validation/design-rules.yaml) at the repository root. This allows committers to add new rules without writing Java code.

Kafka-Specific Rules to Ship With:

  1. consumer-thread-safety:

    • Trigger: KafkaConsumer, consumer.poll

    • Forbidden: ExecutorService, ThreadPoolExecutor, newFixedThreadPool

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

  2. producer-close:

    • Trigger: KafkaProducer, new KafkaProducer

    • Required: producer.close

    • Why: Unclosed producers leak connections and memory.

  3. no-deprecated-metrics:

    • Trigger: MetricName, KafkaMetricsGroup

    • Forbidden: kafka.metrics.KafkaMetricsGroup

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

Implementation Details

Where the code lives: buildSrc/ is Gradle's standard convention for custom build logic. This directory does not currently exist in the Kafka repo, but introducing it is the idiomatic Gradle approach to prevent polluting the root 

buildSrc/
├── 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              # Git diff extraction
    └── 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 Overrides:

  • Emergency bypass: Adding /skip-validation in any commit message skips all gates.

  • Trusted authors: Committers can add their emails to a trusted_authors list in .validation/config.yaml to bypass density checks entirely.

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 buildSrc/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 Rollout): 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. 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.

Rejected Alternatives

  • Relying only on human reviewers: Does not scale. Maintainer time is the scarcest resource.

  • Using third-party GitHub Actions: External binaries introduce supply-chain risk and cannot be run locally. Using buildSrc ensures developers run the exact same checks on their laptops (./gradlew checkContributionQuality).

  • Extending Checkstyle only: Checkstyle operates per-file on the full source. It cannot measure per-PR logic density, nor can it easily enforce conditional architectural rules ("if file contains X, then block Y").

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

FAQ:

Q: Won't failing a CI check generate massive email spam to the dev@kafka mailing list? A: No. To ensure zero spam, the check does not post comments or fail loudly in a way that emails the list. If a PR falls below the density threshold, a GitHub Action silently executes gh pr ready --undo to convert the PR into a Draft and applies a needs-substance label. This removes it from the review queue silently.

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 blocking System.exit), but it struggles with complex, conditional architectural logic. For instance, 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: Won't the Logic Density Ratio (LDR) punish developers who write really good Javadoc? A: No. The script strips out all comments and Javadocs before calculating the ratio. A PR that adds 2 lines of logic and 50 lines of documentation will pass perfectly fine. Furthermore, test directories (src/test/*) and configuration classes are explicitly excluded from the density calculation.

Q: What if the script has a bug and blocks legitimate human contributors? A: The system prioritizes developer velocity. Any developer can bypass the gate immediately by adding /skip-validation to their commit message or PR description. Additionally, the initial 30-day "Shadow Mode" runs non-blocking, allowing the PMC to review its accuracy before it ever gains the power to block a merge.

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.

  • No labels