fix: agent must verify semantic correctness of OS/platform APIs, not just syntax #229

Problem

In PR #232, the agent used resource.getrusage(RUSAGE_SELF).ru_maxrss to measure "current RSS" and "sustained RSS" by periodic sampling. However:

• ru_maxrss is a lifetime high-water mark, not instantaneous RSS. Sampling it every 5 seconds and taking the median gives a meaningless number — it is either the same value or monotonically increasing.
• On macOS, ru_maxrss returns bytes. On Linux, it returns kilobytes. The code used /(1024*1024) everywhere, which is correct only on macOS. CI runs on Linux would report values ~1000× too small.

The unit tests only checked > 0 and is not None, so the platform bug and high-water semantics both passed silently.

Root Cause

1. Did not read the API documentation carefully: ru_maxrss documentation explicitly states "maximum resident set size (in kilobytes on Linux, bytes on macOS)." The agent treated it as current RSS without verification.
2. Platform assumption without a guard: The code was written for M4 macOS but the helpers module (smoke_metrics.py) is imported by tests that run on Linux CI.
3. Test assertions too weak: Checking > 0 for RSS values masks fundamental unit or semantic errors.

Prevention

For the agent

1. Before using any OS/platform API for measurement, verify its semantics. Check: Is this a point-in-time value or a cumulative/high-water value? Is the unit documented? Is it platform-dependent?
2. Add platform guards when using platform-specific APIs. If the measurement is macOS-only, add an explicit check and fail with a clear message on other platforms. If cross-platform, handle the unit difference.
3. Write tests that would catch semantic errors. For measurement APIs, mock the underlying system call and assert the expected conversion. For example, mock resource.getrusage to return a known kilobyte value and verify the helper converts it correctly for the current platform.

For the review checklist

4. Platform-dependent measurement gate: Any PR that uses resource, os.sysconf, /proc/, or similar platform APIs for metrics must document the platform assumption and include platform-aware tests.
5. High-water vs current: If a metric is described as "current" or "sustained," verify the underlying API actually returns a point-in-time value.

Example

Bad (PR #232):

def _rss_mb() -> float:
    # macOS ru_maxrss is bytes, Linux is KB — only correct on macOS
    return resource.getrusage(resource.RUSAGE_SELF).ru_maxrss / (1024 * 1024)

Good:

import platform

def current_rss_mb() -> float | None:
    """Return current process RSS in MB, or None if unavailable."""
    try:
        import psutil
        return psutil.Process().memory_info().rss / (1024 * 1024)
    except ImportError:
        pass
    # Fallback: platform-specific ru_maxrss (high-water, not current)
    import resource
    raw = resource.getrusage(resource.RUSAGE_SELF).ru_maxrss
    divisor = (1024 * 1024) if platform.system() == "Darwin" else 1024
    # Note: this is high-water, not current — caller must label accordingly
    return raw / divisor

References

• PR fix: integration test mandatory for instrumentation and cross-cutting PRs #232 review: https://github.com/Cor-Incorporated/persona-village-v2/pull/232#issuecomment-4525827375
• Python docs: https://docs.python.org/3/library/resource.html#resource.getrusage