feat: support log sampling and remove unused method

[jaronoff97]

No description provided.

[macroscopeapp]

🟡 Medium policy/parser.zig:554

parseValue reads v.string unconditionally when an object contains a string_value key, without checking that the JSON value is actually a string. A policy like {"equals":{"string_value":123}} causes the parser to access the inactive field of std.json.Value, producing a runtime trap or panic during policy loading instead of returning error.InvalidValue. Consider checking v == .string before accessing v.string, and returning error.InvalidValue for non-string values.

Suggested change

	if (obj.get("string_value")) |v| {
	return Value{ .value = .{ .string_value = try allocator.dupe(u8, v.string) } };
	}
	if (obj.get("string_value")) |v| {
	if (v != .string) return error.InvalidValue;
	return Value{ .value = .{ .string_value = try allocator.dupe(u8, v.string) } };
	}

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:

In file @src/policy/parser.zig around lines 554-556:

`parseValue` reads `v.string` unconditionally when an object contains a `string_value` key, without checking that the JSON value is actually a string. A policy like `{"equals":{"string_value":123}}` causes the parser to access the inactive field of `std.json.Value`, producing a runtime trap or panic during policy loading instead of returning `error.InvalidValue`. Consider checking `v == .string` before accessing `v.string`, and returning `error.InvalidValue` for non-string values.

[macroscopeapp]

🟠 High policy/probabilistic_sampler.zig:131

shouldKeepRandom returns true whenever io is null, so unkeyed log percentage sampling silently keeps 100% of records for any configured rate between 1% and 99%. The null-io fail-open path makes the sampler a no-op in the common case where a log policy has no sample_key and the caller omits .io. If failing open is intentional here, consider documenting that an absent io disables sampling so callers know they must supply one for percentage sampling to take effect.

Also found in 1 other location(s)

src/policy/policy_engine.zig:730

findMatchingPolicies now routes unkeyed log percentage sampling through s.shouldKeepRandom(io) when input.len == 0. EvaluateOptions.io is documented as optional and null should only degrade rate limiting / regex redact, but ProbabilisticSampler.shouldKeepRandom(null) fails open to true for any percentage between 0 and 100. As a result, evaluating a log policy with keep: "50%" and no sample_key (or a missing/empty sample key field) while leaving options.io = null keeps every matching record instead of sampling them.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:

In file @src/policy/probabilistic_sampler.zig around line 131:

`shouldKeepRandom` returns `true` whenever `io` is null, so unkeyed log percentage sampling silently keeps 100% of records for any configured rate between 1% and 99%. The null-`io` fail-open path makes the sampler a no-op in the common case where a log policy has no `sample_key` and the caller omits `.io`. If failing open is intentional here, consider documenting that an absent `io` disables sampling so callers know they must supply one for percentage sampling to take effect.

Also found in 1 other location(s):
- src/policy/policy_engine.zig:730 -- `findMatchingPolicies` now routes unkeyed log percentage sampling through `s.shouldKeepRandom(io)` when `input.len == 0`. `EvaluateOptions.io` is documented as optional and null should only degrade rate limiting / regex redact, but `ProbabilisticSampler.shouldKeepRandom(null)` fails open to `true` for any percentage between 0 and 100. As a result, evaluating a log policy with `keep: "50%"` and no `sample_key` (or a missing/empty sample key field) while leaving `options.io = null` keeps every matching record instead of sampling them.

[macroscopeapp]

🟠 High policy/matcher_index.zig:1295

equals.string_value matchers are routed through the Hyperscan exact path, which wraps the literal in ^...$ via formatPattern without escaping regex metacharacters. As a result, a literal equality like "a.b" also matches "axb" (the . matches any character), and valid literals such as "a(b" are rejected during validation as "invalid pattern" because the unescaped ( is invalid regex. This breaks string equality semantics for any value containing regex syntax. Consider escaping the literal before compiling it as an exact pattern, or handling string equality without a regex engine.

Also found in 1 other location(s)

src/policy/parser.zig:534

The new bare-string path lets policies author equals: "", but that value is later routed through the string-pattern path in matcher_index and dropped by the if (pattern.len == 0) return; check. As a result, equality against an empty string silently never matches even though parseValue accepts it as a valid string_value.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:

In file @src/policy/matcher_index.zig around line 1295:

`equals.string_value` matchers are routed through the Hyperscan `exact` path, which wraps the literal in `^...$` via `formatPattern` without escaping regex metacharacters. As a result, a literal equality like `"a.b"` also matches `"axb"` (the `.` matches any character), and valid literals such as `"a(b"` are rejected during validation as "invalid pattern" because the unescaped `(` is invalid regex. This breaks string equality semantics for any value containing regex syntax. Consider escaping the literal before compiling it as an `exact` pattern, or handling string equality without a regex engine.

Also found in 1 other location(s):
- src/policy/parser.zig:534 -- The new bare-string path lets policies author `equals: ""`, but that value is later routed through the string-pattern path in `matcher_index` and dropped by the `if (pattern.len == 0) return;` check. As a result, equality against an empty string silently never matches even though `parseValue` accepts it as a valid `string_value`.

[macroscopeapp]

🟡 Medium policy/log_transform.zig:222

Regex redaction in applyRedact now bails out when accessor.typed_value returns a non-.string value, so a regex redact rule targeting a field exposed as .bytes (e.g. LOG_FIELD_TRACE_ID / LOG_FIELD_SPAN_ID) always returns false and leaves the field unchanged. Previously accessor.value supplied a textual representation that the regex could still match. Consider coercing non-string typed values to text before running the regex, or document that regex redaction intentionally excludes .bytes fields.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:

In file @src/policy/log_transform.zig around line 222:

Regex redaction in `applyRedact` now bails out when `accessor.typed_value` returns a non-`.string` value, so a regex redact rule targeting a field exposed as `.bytes` (e.g. `LOG_FIELD_TRACE_ID` / `LOG_FIELD_SPAN_ID`) always returns `false` and leaves the field unchanged. Previously `accessor.value` supplied a textual representation that the regex could still match. Consider coercing non-string typed values to text before running the regex, or document that regex redaction intentionally excludes `.bytes` fields.

[macroscopeapp]

Approvability

Verdict: Needs human review

2 blocking correctness issues found. This PR introduces new log sampling behavior and breaking API changes (removing the value accessor). Multiple high-severity unresolved review comments identify bugs in the new equals.string_value path and sampling logic that affect correctness of policy evaluation.

^{You can customize Macroscope's approvability policy. Learn more.}