From a135c761e6d583b54de3dbb6fa4ae8f49c8d3dc4 Mon Sep 17 00:00:00 2001
From: torlando-tech <torlando-tech@users.noreply.github.com>
Date: Mon, 4 May 2026 15:43:14 -0400
Subject: [PATCH] =?UTF-8?q?chore(greptile):=20iter=201=20=E2=80=94=20appli?=
 =?UTF-8?q?ed=204=20(3=20ACCEPT=20+=201=20MODIFY),=20rejected=201?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Greptile review feedback on PR #21:

ACCEPT:
- test_patch_nimble.py:151 (P1) — replace dead `if False else True`
  ternary with a real assertion that "already applied" is absent on
  the first run.
- test_patch_nimble.py:247 (P1) — invoke the shim subprocess via
  `sys.executable` instead of hardcoded `/usr/bin/python3` so CI's
  setup-python interpreter is used consistently.
- workflows/test.yml:50 (P2) — include hash of
  deps/microReticulum/platformio.ini in PlatformIO cache key so the
  cache invalidates when dependencies change.

MODIFY (narrowed):
- test_ring_buffers.cpp:209 (P2) — keep both `write(data, 0)` and
  `write(data, -1)` assertions, but add a comment clarifying that
  EncodedRingBuffer::write() takes signed `int length` (not size_t),
  so -1 hits the `length <= 0` branch — same as 0. Greptile's
  premise (size_t wrap to SIZE_MAX) does not apply to this codebase.
  The two assertions lock the contract in case the param is ever
  migrated to size_t.

REJECT (silently — no public reply per agent policy):
- test_audio_filters.cpp:237 (P1) — VoiceFilterChain::process()
  takes `numSamples = frames * channels` per the documented
  contract in audio_filters.h:33-40, and the implementation does
  `numFrames = numSamples / channels_` (audio_filters.cpp:63). The
  multichannel test correctly passes `(int)samples.size() = 8000`
  (4000 frames * 2 channels). No out-of-bounds read occurs.
---
 .github/workflows/test.yml               |  2 +-
 tests/build_scripts/test_patch_nimble.py | 12 ++++++++++--
 tests/native/test_ring_buffers.cpp       |  3 +++
 3 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
index e4e67e65..955ffc07 100644
--- a/.github/workflows/test.yml
+++ b/.github/workflows/test.yml
@@ -47,7 +47,7 @@ jobs:
           path: |
             ~/.cache/pip
             ~/.platformio/.cache
-          key: ${{ runner.os }}-pio-mrn
+          key: ${{ runner.os }}-pio-mrn-${{ hashFiles('deps/microReticulum/platformio.ini') }}
 
       - name: Install PlatformIO
         run: pip install --upgrade platformio
diff --git a/tests/build_scripts/test_patch_nimble.py b/tests/build_scripts/test_patch_nimble.py
index c44c05f3..7d322387 100644
--- a/tests/build_scripts/test_patch_nimble.py
+++ b/tests/build_scripts/test_patch_nimble.py
@@ -15,6 +15,7 @@ What we verify:
 import io
 import os
 import subprocess
+import sys
 from contextlib import redirect_stdout
 from pathlib import Path
 
@@ -148,7 +149,12 @@ def test_pristine_source_applies_all_patches(fresh_tree, patches):
         assert p["label"] in stdout, (
             f"patch {p['label']!r} did not run\n--- stdout ---\n{stdout}"
         )
-        assert f"already applied" not in stdout.split(p["label"])[0].splitlines()[-1] if False else True
+        # On first run, the patch's label line must not say "already applied".
+        label_line = stdout.split(p["label"])[1].split("\n")[0]
+        assert "already applied" not in label_line, (
+            f"first run of {p['label']!r} should not report already-applied\n"
+            f"--- stdout ---\n{stdout}"
+        )
         # File should now contain `new` and not `old`
         content = Path(p["filepath"]).read_text()
         assert p["new"] in content, f"new content missing from {p['filepath']}"
@@ -239,8 +245,10 @@ def test_script_is_executable_via_python(tmp_path):
         f"builtins.env = _Env()\n"
         f"exec(open({str(PATCH_SCRIPT)!r}).read())\n"
     )
+    # Use the same interpreter that runs the test suite (sys.executable),
+    # not /usr/bin/python3 — under setup-python on CI the two diverge.
     result = subprocess.run(
-        ["/usr/bin/python3", str(shim)],
+        [sys.executable, str(shim)],
         capture_output=True,
         text=True,
         timeout=10,
diff --git a/tests/native/test_ring_buffers.cpp b/tests/native/test_ring_buffers.cpp
index 0f14f5cd..5b1217cd 100644
--- a/tests/native/test_ring_buffers.cpp
+++ b/tests/native/test_ring_buffers.cpp
@@ -204,6 +204,9 @@ static void prb_spsc_threaded_stress() {
 static void erb_zero_length_rejected() {
     EncodedRingBuffer eb(4, 64);
     uint8_t data[1] = {0xAA};
+    // EncodedRingBuffer::write() takes `int length` (signed) and rejects
+    // `length <= 0`. Both 0 and -1 hit the same branch — we exercise both
+    // to lock that contract in case the param ever migrates to size_t.
     EXPECT_TRUE(!eb.write(data, 0));
     EXPECT_TRUE(!eb.write(data, -1));
 }