Skip to content

Address code review feedback for build-and-summarize - #775

Draft
jbachorik wants to merge 1 commit into
mainfrom
jb/fix-build-and-summarize
Draft

Address code review feedback for build-and-summarize#775
jbachorik wants to merge 1 commit into
mainfrom
jb/fix-build-and-summarize

Conversation

@jbachorik

Copy link
Copy Markdown
Collaborator

What does this PR do?:

Fixes three bugs in the deterministic Gradle log parser inside .claude/commands/build-and-summarize (added in an uncommitted local change prior to this PR, replacing a headless-claude-spawn approach with a self-contained Python parser):

  • An IndexError when a stray non-blank, non-* line appears between the FAILURE: banner and the * What went wrong: header, leaving headlines empty at index time.
  • Test/module counts being misattributed under parallel Gradle builds, since they were attached to "whatever > Task line was last seen," including non-test tasks that interleave with a test task's completion line.
  • Failure headlines being paired with failed tasks by list position instead of identity, so with --continue and multiple failing tasks, headlines could land on the wrong task or go missing entirely.

Also removes an unverifiable comment claiming managed settings pin a specific model and bill real money for log parsing — that claim isn't backed by anything in this file.

Motivation:

A code review of the uncommitted parser changes surfaced these correctness bugs before they shipped.

Additional Notes:

None.

How to test the change?:

Verified by hand-running the parser logic against a synthetic Gradle log that reproduces all three bugs (stray line before a headline, parallel task interleaving, multi-failure --continue output) and confirming correct output with no crash.

For Datadog employees:

  • This PR doesn't touch any of that.
  • JIRA: N/A

@jbachorik jbachorik added the AI label Sep 1, 2026
@jbachorik jbachorik changed the title Fix build-and-summarize log-parsing bugs Address code review feedback for build-and-summarize Sep 1, 2026
Fixes an IndexError on stray log lines, test-count misattribution
under parallel builds, and positional (rather than identity-based)
pairing of failure headlines with failed tasks. Also removes an
unverifiable comment about managed-settings model billing.
@jbachorik
jbachorik force-pushed the jb/fix-build-and-summarize branch from dc33515 to 30357ff Compare September 1, 2026 15:46
@dd-octo-sts

dd-octo-sts Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Scan-Build Report

User:runner@runnervmgx7h7
Working Directory:/home/runner/work/java-profiler/java-profiler/ddprof-lib/src/test/make
Command Line:make -j4 all
Clang Version:Ubuntu clang version 18.1.3 (1ubuntu1)
Date:Tue Sep 1 15:49:23 2026

Bug Summary

Bug TypeQuantityDisplay?
All Bugs1
Logic error
Dereference of null pointer1

Reports

Bug Group Bug Type ▾ File Function/Method Line Path Length
Logic errorDereference of null pointerfaultInjection.cppcrashNow242

@dd-octo-sts

dd-octo-sts Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

CI Test Results

Run: #33527785367 | Commit: 3fa7d84 | Duration: 14m 44s (longest job)

All 32 test jobs passed

Status Overview

JDK glibc-aarch64/debug glibc-amd64/debug musl-aarch64/debug musl-amd64/debug
8 - - -
8-ibm - - -
8-j9 - -
8-librca - -
8-orcl - - -
11 - - -
11-j9 - -
11-librca - -
17 - -
17-graal - -
17-j9 - -
17-librca - -
21 - -
21-graal - -
21-librca - -
25 - -
25-graal - -
25-librca - -

Legend: ✅ passed | ❌ failed | ⚪ skipped | 🚫 cancelled

Summary: Total: 32 | Passed: 32 | Failed: 0


Updated: 2026-09-01 16:03:20 UTC

@dd-octo-sts

dd-octo-sts Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

All 39 integration tests passed

📊 Dashboard · 👷 Pipeline · 📦 30357ffc

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant