Skip to content

Commit 75e7a52

Browse files
committed
fix(cpp): address PR #858 review comments
- test_integration_vlm.cpp: replace GTEST_MESSAGE_(kNonFatalFailure) in expectPinnedLemonadeVersion() with std::cerr + SUCCEED() so missing version field or /system-info probe failure is purely informational and does not mark VLM tests as failed; real version mismatches still fail via EXPECT_EQ (blocking concern from review) - types.cpp: expand detectImageMimeType short-buffer comment to explain why "image/png" fallback exists for null/<12 byte buffers (AC-15e contract, test fixtures) vs empty-string return for full-sized unrecognized magic (non-blocking concern from review) - types.cpp: add BMP "BM" false-positive trade-off comment (minor nit) - .gitattributes: mark cpp/tests/fixtures/*.png and *.jpg as binary to prevent LF/CRLF text-filter corruption (minor nit)
1 parent 266a4ca commit 75e7a52

3 files changed

Lines changed: 22 additions & 7 deletions

File tree

.gitattributes

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
# Binary test fixtures — prevent LF/CRLF mangling
2+
cpp/tests/fixtures/*.png binary
3+
cpp/tests/fixtures/*.jpg binary

cpp/src/types.cpp

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,13 @@ namespace gaia {
1313

1414
std::string detectImageMimeType(const std::uint8_t* data, std::size_t size) {
1515
// Buffers < 12 bytes cannot be safely probed for WebP (offset 8–11).
16-
// Return "image/png" so callers treating these as padding/header stubs
17-
// don't crash (AC-15e short-buffer safety contract).
16+
// Returning "image/png" for null/short buffers is the AC-15e contract:
17+
// test fixtures call this function directly with 1/5/11-byte header stubs
18+
// and assert the safe fallback. In practice, Image::fromBytes already rejects
19+
// empty byte vectors before calling this, and Image::fromFile reads the full
20+
// file first — so neither factory can produce a mislabeled Image via this
21+
// path. Full-sized buffers (>= 12 bytes) with unrecognized magic return ""
22+
// so callers throw with a clear message.
1823
if (data == nullptr || size < 12) {
1924
return "image/png";
2025
}
@@ -38,6 +43,8 @@ std::string detectImageMimeType(const std::uint8_t* data, std::size_t size) {
3843
return "image/webp";
3944
}
4045
// BMP: "BM"
46+
// Note: "BM" is a 2-byte signature with false-positive potential (any data starting "BM...").
47+
// Acceptable here because Image::fromBytes enforces an explicit whitelist and callers supply real image files.
4148
if (data[0] == 'B' && data[1] == 'M') {
4249
return "image/bmp";
4350
}

cpp/tests/integration/test_integration_vlm.cpp

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
#include <gaia/types.h>
2626

2727
#include <cstdlib>
28+
#include <iostream>
2829
#include <string>
2930

3031
#ifndef GAIA_TEST_FIXTURES_DIR
@@ -94,18 +95,22 @@ void expectPinnedLemonadeVersion() {
9495
v = info["lemonade_version"].get<std::string>();
9596
}
9697
if (v.empty()) {
97-
GTEST_MESSAGE_("Lemonade /system-info did not return a version field; skipping pin check",
98-
::testing::TestPartResult::kNonFatalFailure);
98+
std::cerr << "[VLM version pin] Lemonade /system-info did not return a "
99+
"version field; skipping pin check (informational only)."
100+
<< std::endl;
101+
SUCCEED();
99102
return;
100103
}
101104
EXPECT_EQ(v, std::string(GAIA_PINNED_LEMONADE_VERSION))
102105
<< "Lemonade server version (" << v << ") does not match "
103106
<< "GAIA-pinned version (" << GAIA_PINNED_LEMONADE_VERSION << "). "
104107
<< "Update LEMONADE_VERSION in src/gaia/version.py or roll back the server.";
105108
} catch (const std::exception& e) {
106-
GTEST_MESSAGE_(("Lemonade /system-info probe failed: "
107-
+ std::string(e.what())).c_str(),
108-
::testing::TestPartResult::kNonFatalFailure);
109+
std::cerr << "[VLM version pin] Lemonade /system-info probe failed: "
110+
<< e.what()
111+
<< " (informational only; continuing VLM test)."
112+
<< std::endl;
113+
SUCCEED();
109114
}
110115
}
111116

0 commit comments

Comments
 (0)