Skip to content

chore(baseline-205): tighten excluded-libs scan to file field only#221

Merged
zackees merged 1 commit intomainfrom
chore/baseline-205-tighten-excluded-libs-scan
May 10, 2026
Merged

chore(baseline-205): tighten excluded-libs scan to file field only#221
zackees merged 1 commit intomainfrom
chore/baseline-205-tighten-excluded-libs-scan

Conversation

@zackees
Copy link
Copy Markdown
Member

@zackees zackees commented May 10, 2026

Summary

The previous scan in parse_compile_commands matched the needles (FNET / Snooze / RadioHead / mbedtls) against every field of every compile_commands.json entry — file + directory + command + arguments. That caught the framework-wide -I.../libraries/<lib> header search-path flag on every TU, so teensylc's report read "FNET: 68 entries" just because the include path was on every translation unit. The actual AC#1 question — "did any FNET source file get compiled?" — was masked.

Switched to a path-segment match (/libraries/<needle>/) against the file field only. Now the count is "TUs whose file is a <needle>/... source file" — the question AC#1 asks.

Result

All four boards now report 0 (not compiled) for every excluded library:

env TUs FNET Snooze RadioHead mbedtls
teensylc 68 0 0 0 0
teensy30 68 0 0 0 0
teensy41 85 0 0 0 0
stm32f103c8 201 0 0 0 0

AC#1 is met cleanly across the matrix — no hand-reading required.

The report line was also reworded from "Excluded library hits in compile_commands.json" → "Excluded-library source files compiled (AC#1 must be 0 for all)" so the report header states the contract directly.

Test plan

  • Sanity-check: ran the new parser against the existing teensylc compile_commands.json — 68 TUs, all zeros. Cross-verified by hand-counting file fields under /libraries/<needle>/ (also zero).
  • Re-ran the full baseline; all 4 boards report 0 for every needle.

Refs: #205 AC#1.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Chores

    • Refined baseline measurement collection to more accurately track compilation units across board targets.
  • Documentation

    • Completed baseline-205 documentation with captured measurement data, including per-board metrics and build status.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 10, 2026

Warning

Rate limit exceeded

@zackees has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 56 minutes and 58 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 0659cdb2-5f87-4e04-aa18-7a5856097968

📥 Commits

Reviewing files that changed from the base of the PR and between 0e4ea8d and f80f732.

📒 Files selected for processing (2)
  • ci/measure_baseline_205.py
  • tasks/baseline-205.md
📝 Walkthrough

Walkthrough

The PR refines baseline-205 measurement collection by correcting the Teensy LC environment identifier to lowercase, updating library-hit detection to scan only compile_commands.json file fields with normalized path matching, and capturing fresh measurements across four target boards with the refined output format.

Changes

Baseline-205 Measurement Refinement

Layer / File(s) Summary
Target Registry & Documentation
ci/measure_baseline_205.py
Target environment name changed from teensyLC to teensylc; docstrings updated to document that library hits are detected only from file-field paths matching normalized /libraries/<lib>/ patterns.
Library Hit Detection Logic
ci/measure_baseline_205.py
Library-hit detection refactored to scan only the file field with normalized path separators and /libraries/<needle>/ matching, replacing prior multi-field substring scanning.
Output Formatting
ci/measure_baseline_205.py
Markdown output labels excluded libraries as 0 (not compiled) or <count> TU(s) compiled.
Captured Measurements
tasks/baseline-205.md
Baseline measurements captured across teensylc, teensy30, teensy41, and stm32f103c8 with timestamped metadata, per-board TU counts and section sizes, excluded-library scans, and build-status summary table.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

A baseline refined, a name made lowercase,
Library paths now scanned with proper grace—
Four boards measured, their footprints captured true,
The measurement script works as intended to do! 🐰✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main technical change: tightening the excluded-libs scan to match only the file field instead of searching all fields.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch chore/baseline-205-tighten-excluded-libs-scan

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

The previous scan globbed every needle (FNET / Snooze / RadioHead /
mbedtls) against the entire compile_commands.json entry — file +
directory + command + arguments. That caught the framework-wide
`-I.../libraries/<lib>` header search-path flag on every TU, so the
report read "FNET: 68 entries" on teensylc just because the include
path was on every translation unit. The actual AC#1 question — "did
any FNET source file get compiled?" — was masked.

Switch to a path-segment match against the `file` field only:
`/libraries/<needle>/` in the normalized POSIX form. Now the count is
"TUs whose `file` is a `<needle>/...` source file", which is the
question AC#1 asks.

With the fix, all four boards (teensylc, teensy30, teensy41,
stm32f103c8) report `0 (not compiled)` for every excluded library —
AC#1 is met cleanly across the matrix, no hand-reading required.

Also reworded the report line from "Excluded library hits in
compile_commands.json" → "Excluded-library source files compiled
(AC#1 must be 0 for all)" so the report header itself states the
contract.

Refs: #205 AC#1.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@zackees zackees force-pushed the chore/baseline-205-tighten-excluded-libs-scan branch from 0e4ea8d to f80f732 Compare May 10, 2026 15:43
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
ci/measure_baseline_205.py (1)

388-390: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add language identifier to generated code block.

The generated markdown code block is missing a language identifier, causing a markdownlint warning. This affects the generated tasks/baseline-205.md file.

📝 Proposed fix
-        lines.append("```")
+        lines.append("```shell")
         lines.append("uv run python ci/measure_baseline_205.py --out tasks/baseline-205.md")
         lines.append("```")
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@ci/measure_baseline_205.py` around lines 388 - 390, Update the code that
emits the markdown fenced code block so it includes a language identifier:
change the call that appends the opening fence (the lines.append("```")
invocation that outputs the uv run example in ci/measure_baseline_205.py) to
append a language-tagged fence (e.g., "```shell") so the generated
tasks/baseline-205.md contains ```shell before the command and fixes the
markdownlint warning; leave the closing lines.append("```") untouched.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@ci/measure_baseline_205.py`:
- Around line 388-390: Update the code that emits the markdown fenced code block
so it includes a language identifier: change the call that appends the opening
fence (the lines.append("```") invocation that outputs the uv run example in
ci/measure_baseline_205.py) to append a language-tagged fence (e.g., "```shell")
so the generated tasks/baseline-205.md contains ```shell before the command and
fixes the markdownlint warning; leave the closing lines.append("```") untouched.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 5dd640fb-109f-4304-bac1-78a3821b546f

📥 Commits

Reviewing files that changed from the base of the PR and between 6b7fb4b and 0e4ea8d.

📒 Files selected for processing (2)
  • ci/measure_baseline_205.py
  • tasks/baseline-205.md

@zackees zackees merged commit 787cc82 into main May 10, 2026
77 checks passed
zackees added a commit that referenced this pull request May 10, 2026
…nsylc env, add CI badges

Three final polish items before closing #205.

## test — `r04_pass2_reconciliation_catches_cpp_only_dependency`

The canonical "why 2-pass beats single-pass BFS" scenario, which the
existing 16-test suite was missing as an explicit guard:

- Project includes `<SPI.h>`. `SPI.h` is silent (no transitive includes).
- `SPI.cpp` is the *only* place that includes `<Wire.h>`.

A single-pass BFS over headers selects {SPI} and silently misses Wire —
link-time undefined symbols. The LDF's pass 2 re-seeds with each
selected lib's full source set, so Wire is reached and selected. Test
asserts {SPI, Wire} and is the regression guard against accidentally
collapsing the resolver back to a single pass. Brings the resolver test
count to 17 (10 LDF + 7 cache).

## fix — teensylc acceptance gate env name

`tests/teensylc_acceptance.rs` passed `env_name: "teensyLC"`
(camelCase) and looked up `compile_commands.json` under
`<build>/teensyLC/`. The actual env in
`tests/platform/teensylc/platformio.ini` is `[env:teensylc]`
(lowercase) — same case-sensitivity bug fixed in #220 / #221 for
`measure_baseline_205.py`. The acceptance gate has been failing on
every run (including main) for this reason.

## badges

Adds badges for the two #205 workflows to README.md's CI status block:

- `acceptance-205.yml` — AC#1 (teensyLC) and AC#4 (stm32 SPI) gates
- `bench-205.yml` — P-02 (cold) / P-03 (scanner) / P-01-mini (warm)
  perf gates

Both have been live since #211 / #210 but were not discoverable from
the README.

Refs: #205.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
zackees added a commit that referenced this pull request May 10, 2026
…nsylc env, add CI badges

Three final polish items before closing #205.

## test — `r04_pass2_reconciliation_catches_cpp_only_dependency`

The canonical "why 2-pass beats single-pass BFS" scenario, which the
existing 16-test suite was missing as an explicit guard:

- Project includes `<SPI.h>`. `SPI.h` is silent (no transitive includes).
- `SPI.cpp` is the *only* place that includes `<Wire.h>`.

A single-pass BFS over headers selects {SPI} and silently misses Wire —
link-time undefined symbols. The LDF's pass 2 re-seeds with each
selected lib's full source set, so Wire is reached and selected. Test
asserts {SPI, Wire} and is the regression guard against accidentally
collapsing the resolver back to a single pass. Brings the resolver test
count to 17 (10 LDF + 7 cache).

## fix — teensylc acceptance gate env name

`tests/teensylc_acceptance.rs` passed `env_name: "teensyLC"`
(camelCase) and looked up `compile_commands.json` under
`<build>/teensyLC/`. The actual env in
`tests/platform/teensylc/platformio.ini` is `[env:teensylc]`
(lowercase) — same case-sensitivity bug fixed in #220 / #221 for
`measure_baseline_205.py`. The acceptance gate has been failing on
every run (including main) for this reason.

## badges

Adds badges for the two #205 workflows to README.md's CI status block:

- `acceptance-205.yml` — AC#1 (teensyLC) and AC#4 (stm32 SPI) gates
- `bench-205.yml` — P-02 (cold) / P-03 (scanner) / P-01-mini (warm)
  perf gates

Both have been live since #211 / #210 but were not discoverable from
the README.

Refs: #205.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
zackees added a commit that referenced this pull request May 10, 2026
…nsylc env, add CI badges (#222)

Three final polish items before closing #205.

## test — `r04_pass2_reconciliation_catches_cpp_only_dependency`

The canonical "why 2-pass beats single-pass BFS" scenario, which the
existing 16-test suite was missing as an explicit guard:

- Project includes `<SPI.h>`. `SPI.h` is silent (no transitive includes).
- `SPI.cpp` is the *only* place that includes `<Wire.h>`.

A single-pass BFS over headers selects {SPI} and silently misses Wire —
link-time undefined symbols. The LDF's pass 2 re-seeds with each
selected lib's full source set, so Wire is reached and selected. Test
asserts {SPI, Wire} and is the regression guard against accidentally
collapsing the resolver back to a single pass. Brings the resolver test
count to 17 (10 LDF + 7 cache).

## fix — teensylc acceptance gate env name

`tests/teensylc_acceptance.rs` passed `env_name: "teensyLC"`
(camelCase) and looked up `compile_commands.json` under
`<build>/teensyLC/`. The actual env in
`tests/platform/teensylc/platformio.ini` is `[env:teensylc]`
(lowercase) — same case-sensitivity bug fixed in #220 / #221 for
`measure_baseline_205.py`. The acceptance gate has been failing on
every run (including main) for this reason.

## badges

Adds badges for the two #205 workflows to README.md's CI status block:

- `acceptance-205.yml` — AC#1 (teensyLC) and AC#4 (stm32 SPI) gates
- `bench-205.yml` — P-02 (cold) / P-03 (scanner) / P-01-mini (warm)
  perf gates

Both have been live since #211 / #210 but were not discoverable from
the README.

Refs: #205.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.

1 participant