Commit 3fe7389
authored
fix(redis): cascade-safety timeouts + per-command observability + cross-engine doc parity (#137)
* fix(pine-go): expose Redis cascade-safety timeouts on redis_connection resource
Adds five parameters to the redis_connection resource schema:
dial_timeout_ms default 2000 TCP dial
read_timeout_ms default 2000 per-command read (primary cascade knob)
write_timeout_ms default 2000 per-command write
pool_timeout_ms default 2000 wait for free pool conn
pool_size default 0 0 = client default (10*GOMAXPROCS)
These are wired through redisOptionsFromParams (split out so tests can
verify the mapping without miniredis) into go-redis's *redis.Options.
Background — 2026-06-22 tipsy-recsys incident:
A brief Redis hiccup (PING p99 ~970ms for ~5min) triggered a 30-minute
production outage. Pre-fix the resource constructed redis.NewClient with
only Addr/Password/DB, inheriting v9 defaults (read/write 3s, dial 5s,
PoolSize 10*GOMAXPROCS). With 2 vCPU containers PoolSize=20, which the
incident exhausted (Redis Pool Connections grafana panel went 5→20 the
moment PING latency rose). Each /execute request stuck in fetch_realshow
or filter_blocked_creators redis calls held its goroutine + RowFrame +
OperatorOutput in heap until the 10s HTTP timeout. Upstream retried,
the goroutine pile-up doubled, heap_inuse climbed RSS = Go sys = 3.87
GiB until OOM. Goroutine sched latency P99 hit 117ms (heap-pressured
runtime, not GC — GC frequency actually dropped because alloc rate
fell as goroutines parked on Redis I/O).
Why 2s defaults rather than 500ms:
The defaults must not regress healthy deployments. 2s is below v9's
3s/5s but well above any healthy steady-state (recsys's measured
PING p50 is sub-ms; even ZRANGEBYSCORE on 24h impression history is
single-digit ms). Operators that have measured their server's healthy
P99 should configure tighter values explicitly — recsys is moving to
500ms read/write under a separate change.
The redis operators (transform_redis_get/set, transform_redis_zrangebyscore
and recsys's own transform_query_blocked_creators) all default
fail_on_error=false, so a ctx.DeadlineExceeded from these timeouts
degrades the call into a cache-miss + warning rather than failing the
request. Cascade containment is at the resource boundary; downstream
business logic continues with cache-miss defaults.
Test:
- TestRedisOptionsFromParams_Defaults: all 4 timeouts land at 2s,
pool_size=0 preserves client default behaviour, metrics_name="".
- TestRedisOptionsFromParams_Explicit: every user-supplied value
flows through to *redis.Options verbatim (the regression test
that would have caught the original gap).
- TestRedisOptionsFromParams_RequireAddr: addr-required guard.
- Existing miniredis tests still green.
Refs: 2026-06-22 incident postmortem (recsys for_you).
* fix(pine-java): expose Redis cascade-safety timeouts on redis_connection resource
Mirrors the pine-go change: adds dial_timeout_ms / read_timeout_ms /
write_timeout_ms / pool_timeout_ms / pool_size to the redis_connection
resource schema, all with default 2000 ms (pool_size default 0 = pool
default), and threads them through to JedisPool via
DefaultJedisClientConfig + GenericObjectPoolConfig.
Jedis-specific shape notes:
- Jedis exposes connect timeout and a single socket timeout (no
separate read/write knobs). write_timeout_ms is accepted in the
schema for cross-engine parity but mapped to max(read_timeout_ms,
write_timeout_ms) for the underlying socket so a strict
write_timeout below the read deadline doesn't silently turn
long-running reads (LRANGE / ZRANGEBYSCORE) into write failures.
- pool_timeout_ms maps to GenericObjectPoolConfig.setMaxWait. pool_size
maps to setMaxTotal, with setMaxIdle lifted to the same value when
pool_size > 0 so ephemeral peaks aren't immediately dropped after
use; pool_size=0 leaves the commons-pool2 default in place.
- Pre-fix the resource constructed JedisPool with a hard-coded 2000ms
connect timeout and no socket timeout, plus the default
JedisPoolConfig (maxTotal=8). socket-timeout=∞ was the dominant
cascade gap on Java just as v9 default 3s was on Go.
Test:
schemaExposesCascadeSafetyParams locks the cross-engine-shared
defaults (2000 ms × 4, pool_size=0) on the registered schema,
matching pine-go's TestRedisOptionsFromParams_Defaults.
Refs: 2026-06-22 incident postmortem (recsys for_you).
* fix(pine-cpp): expose Redis cascade-safety timeouts on redis_connection resource
Mirrors the pine-go and pine-java changes: adds dial_timeout_ms /
read_timeout_ms / write_timeout_ms / pool_timeout_ms / pool_size to
the redis_connection resource schema. read/write timeouts use
SO_RCVTIMEO/SO_SNDTIMEO; dial_timeout uses a nonblocking-connect +
select() loop so it actually bounds the round trip rather than
relying on the kernel's TCP retransmit fallback.
C++-specific shape notes:
- Pre-fix the redis Client hard-coded a 2s socket timeout on both
read and write, and connect() was blocking with no application
timeout (subject to the kernel default ~75-130s). dial_timeout is
now applied via the standard nonblocking-connect dance: switch the
fd to O_NONBLOCK, fire connect, wait on writable with select bounded
by the deadline, check SO_ERROR, restore blocking.
- pool_timeout_ms is accepted on the schema for cross-engine parity
but is currently a no-op in C++. The pool's acquire never blocks
on a borrow — it constructs a fresh Client when the per-key idle
queue is empty — so there is no wait deadline to bound. Documented
on the schema; if a future refactor caps total concurrent
connections, this knob becomes the wait deadline.
- pool_size maps to the per-key idle-queue cap (was hard-coded
kMaxIdlePerKey=16). pool_size=0 keeps that legacy default; >0
overrides via ConnectionPool::set_max_idle_per_key. The C++ pool
has no hard ceiling on total concurrent clients per key — only this
idle-queue cap — so the semantics differ slightly from Go's
PoolSize ceiling. The schema is symmetric across engines; the
effective bound is the engine's strictest one.
Test:
- "redis_connection schema exposes cascade-safety defaults" locks
the cross-engine-shared defaults (2000 ms x 4, pool_size=0) on
the registered schema, matching pine-go's
TestRedisOptionsFromParams_Defaults and pine-java's
schemaExposesCascadeSafetyParams.
- 214/214 pine_cpp_tests still green.
Refs: 2026-06-22 incident postmortem (recsys for_you).
* chore(codegen): regenerate apple_generated/resources.py for Redis timeout schema
Picks up the dial_timeout_ms / read_timeout_ms / write_timeout_ms /
pool_timeout_ms / pool_size params added to the redis_connection
resource schema across pine-go / pine-java / pine-cpp.
The Python DSL now accepts these as kwargs on RedisConnectionResource:
flow.resource("redis_conn", RedisConnectionResource(
addr=_redis["host"],
password=_redis.get("password", ""),
metrics_name="recsys",
read_timeout_ms=500, # cap any single redis call at 500ms
write_timeout_ms=500,
dial_timeout_ms=1000,
pool_timeout_ms=1000,
pool_size=50,
))
Generated by `scripts/codegen.sh` (Go backend); cross-validate is green
across all three engines.
Refs: 2026-06-22 incident postmortem (recsys for_you).
* fix(pine-java): align codegen doc rendering with pine-go (Type case + default literal)
Three rendering drifts surfaced when round-tripping the redis_connection
schema change through scripts/codegen.sh --backend java:
Drift Go (canonical) Java (pre-fix)
─────────────────────────────── ────────────────── ────────────────
**Type** heading `Transform` `transform`
String default in param table `` `"string"` `` `` `string` ``
Bool default in param table `` `False` `` `` `false` ``
Root causes:
- fromRegistry() used `enum.name().toLowerCase()`, producing all-lowercase
("transform") whereas Go's OperatorType constants are PascalCase
("Transform"). Replaced with pascalCaseEnum() that splits on "_" so
future multi-word types (e.g. MERGE_DEDUP_UNION → MergeDedupUnion)
render cleanly.
- Per-operator markdown was rendering `spec.defaultValue` via implicit
toString(), bypassing the existing toPythonLiteral() helper. Bare
toString() produces "string" (no quotes) and "false" (lowercase),
whereas Go's pythonLiteral uses fmt.Sprintf("%q") for strings and
emits "True"/"False" for bools. Wired toPythonLiteral() into the
doc render path so a single helper now governs both Python code
and markdown rendering.
Why nobody saw this before:
- Production codegen runs use the Go backend (default of codegen.sh).
- cross-validate's 01-codegen-schema.sh checks structural schema
parity (param types/required/defaults) and byte-level Python
output parity, but never compared the generated markdown across
engines, so the Java backend's drift sat dormant.
Why pine-cpp is fine:
- pine-cpp's pineapple-codegen has no markdown emit path (only
--schema-json export and Python codegen). The Python emit's
python_literal() already mirrors Go (True/False, string-quoted),
and 01-codegen-schema.sh's `Go vs C++ Python output (byte-level
match)` check has been guarding this since it was added.
Test:
CodegenDocParityTest pins:
- pascalCaseEnum on each registered OperatorType + multi-word
defensive case + null/empty edge cases.
- toPythonLiteral on string (quoted), bool (capitalised),
integer (no decimal), null (None).
243/243 pine-java tests green (was 236, +7 from the new test).
cross-validate all green (no real FAILs).
Refs: 2026-06-22 incident postmortem (recsys for_you).
* chore: bump version 0.10.9 → 0.10.10
* fix(pine-cpp): contain Redis Client connect/AUTH failures within connected()=false
Two fixes around Client construction so Redis flakiness stays at the operator
boundary (fail_on_error=false → cache miss + warning) instead of escalating
into a request abort or process abort.
(1) AUTH / SELECT failure now sets connected()=false instead of throwing
Pre-fix the Client ctor would let std::runtime_error from expect_ok() unwind
out of std::make_unique<Client> in ConnectionPool::acquire and tear down
the request. The dial-timeout / connect-failure paths above already silently
fall through to fd_ = -1, so AUTH/SELECT was the asymmetric path. The 2026-
06-22 review (.code-review/from-24975c2/...) flagged this as the contract
mismatch with the Redis section of llmdoc/reference/operator-contract.md.
Fix: wrap AUTH/SELECT in try/catch, close fd_ on exception, leave the rest
of the protocol-error throws in place (PING / read_line / read_bulk_string
etc.) — those are during normal command execution and the operator already
catches them.
(2) send_command now uses send(MSG_NOSIGNAL) instead of write()
A Redis server that closes the connection mid-write (RST / transient broker
restart) would deliver SIGPIPE to the process. The cascade-safety contract
for this resource is that Redis flakiness must never escalate beyond the
operator. SIGPIPE'ing the whole pineapple-server is the antithesis of that;
the change turns the dropped peer into a normal throw, which (1) above and
the operator-level fail_on_error=false branch surface as a warning.
Surfaced while writing the regression test for (1) — the test server closes
its accept'd socket immediately, the client's AUTH write hits a closed peer,
and pre-fix the test process aborted with SIGPIPE before it could even check
connected(). pine-cpp/src/server/server.cpp:461 already used MSG_NOSIGNAL
for the same reason; the redis client was the missing peer.
(3) Documentation: connect_with_timeout's failure contract
Made explicit in the function-level comment that on false the caller MUST
close the fd and that the socket may still be in non-blocking mode. Today
the sole caller in this file does close fd on false (the only safe pattern),
but the contract was implicit; pinning it heads off future drift.
Test:
- redis Client AUTH/SELECT failure marks connected=false instead of
throwing — binds a 127.0.0.1 ephemeral listener, accepts then closes
immediately, constructs Client with non-empty password (forcing AUTH).
Pre-fix the ctor threw out of the test (verified by temporarily
disabling the try/catch — got SIGABRT). Post-fix connected()==false
and the test joins cleanly.
- 215/215 pine_cpp_tests green.
Refs: 2026-06-22 incident postmortem (recsys for_you), code review
.code-review/from-24975c2/from-24975c2-to-77b6e18.md.
* fix(redis): pin Java write_timeout policy + surface engine-specific note in schema
The 2026-06-22 review (.code-review/from-24975c2/...) flagged that pine-java's
single-socket-timeout mapping is asymmetric with Go and C++:
Go / C++: read_timeout and write_timeout honoured independently
(separate SO_RCVTIMEO / SO_SNDTIMEO; redis.Options).
Java: Jedis exposes only socketTimeoutMillis. Currently mapped as
max(read_timeout_ms, write_timeout_ms) so reads that wait on
the server (LRANGE / ZRANGEBYSCORE) aren't capped at write_timeout.
Two issues with the original:
(1) The trade-off was buried in a comment at the call site only — users
looking at the schema description (or the Apple Python codegen-emitted
docstring) would never know. Configuring read_timeout_ms=500,
write_timeout_ms=2000 produces a 500 ms wall on Go but 2000 ms on
Java, which is the exact "surprise" cascade-safety is meant to prevent.
(2) The policy choice (max vs min) was a private detail with no test
coverage. A future refactor could flip it to min — which would silently
turn long-running reads into write-timeout failures — and only
production traffic would catch the drift.
Fixes:
- Extract the mapping into AllOperators.effectiveSocketTimeoutMs(read, write),
package-private, with a Javadoc that explains why max wins.
- Update the write_timeout_ms schema description on all three engines
(Go / Java / C++) to surface the Java-specific note. Apple Python codegen
reads the Go schema by default, so the Python docstring now carries the
warning. The note also lands in pine-cpp's redis_connection.cpp comment
block at the top of the file for consistency.
- New regression tests on RedisConnResourceTest:
* writeTimeoutMapsToMaxOfReadAndWriteOnJedis — pins the helper output
on symmetric / asymmetric / default inputs.
* writeTimeoutSchemaDescriptionMentionsMaxBehaviour — pins the
description language so a future drift on policy is visible at
config-time.
6/6 RedisConnResourceTest green; 245/245 pine-java tests overall.
cross-validate green (no real FAILs).
Refs: 2026-06-22 incident postmortem (recsys for_you), code review
.code-review/from-24975c2/from-24975c2-to-77b6e18.md.
* refactor(codegen): unify required-first param ordering + close Java/Go doc drifts
The 2026-06-22 review (.code-review/from-24975c2/...) flagged three drifts
between Go and Java markdown emitters and a deeper UX paper-cut: alphabetical
param ordering (`addr, db, dial_timeout_ms, metrics_name, password, ...`)
visually mixed mandatory and optional knobs together. With the new five
Redis-cascade params the chaos is acute; this commit aligns the three engines
on a tier-then-alphabet ordering.
Changes:
(1) sortedParams now returns required-first, then optional, alphabetised
within each tier. pine-go/pkg/codegen/template.go and pine-java's
Codegen.java grow a shared sortedParams helper; pine-cpp's
cmd/pineapple-codegen/main.cpp gets sorted_param_names with the same
semantics. operators.py and resources.py now lead with required keys
(e.g. lua_script, host+port, key_prefix+resource_name) — readers see
the must-supply contract first. The byte-level Python output stays
comparable across all three engines (cross-validate 1c/1d gates green).
(2) pine-java Codegen no longer drops absent Metadata Contract rows. Pre-fix
operators that declared only ItemOutput (e.g. recall_resource) emitted
a single-row table; pine-go's BacktickWrap renders empty fields as "-"
so all four rows are always present. Java's markdown now renders the
same shape via a small backtickOrDash helper.
(3) scripts/codegen.sh --backend java now passes -ops-dir to Codegen so the
javadoc-driven Metadata Contract parsing runs. Pre-fix the path was
dormant — Java markdown silently omitted every Metadata Contract section
because metadataDocs was always empty.
(4) Schema description string fixes in pine-java/AllOperators.java:
transform_copy.direction, merge_dedup.strategy, reorder_sort.order,
transform_redis_{get,set}.data_type, observe_log description. All
carried subtly different language from pine-go (missing surrounding
quotes around enum values, abbreviated observe_log description).
Aligning them is what made the Go-vs-Java doc diff converge.
(5) README.md trailing whitespace: pine-java now emits the same blank-line
boundary as pine-go's index template (one blank between description and
first category, trailing newline at EOF).
(6) llmdoc/reference/operator-contract.md Redis section now lists all
eleven redis_connection params (was six pre-PR), surfaces the
pine-java max(read,write) note, and flags the pine-cpp pool_timeout_ms
no-op + pool_size semantic difference. Also adds the cascade-safety
background sentence pointing back to the 2026-06-22 incident.
What's intentionally NOT in this commit:
- pine-cpp doc emit. cpp's pineapple-codegen has no markdown path today and
cannot synthesize the Metadata Contract section without source-comment
parsing or a schema-side MetadataDoc field. The follow-up commit lifts
metadata onto OperatorSchema directly so all three engines render from
a single source of truth, then cross-validate gains a Go-Java-cpp
markdown byte-equal gate.
- Go-Java markdown byte-equal gate in cross-validate. Will land in the
same follow-up alongside pine-cpp doc emit so the gate covers all
three engines symmetrically.
Test:
- go vet + go test ./operators/transform/ -run TestRedis green
- mvn test green (245 tests; +2 RedisConnResourceTest cases for the
write_timeout policy lock from the previous commit)
- pine_cpp_tests 215/215 green
- cross-validate green (no real FAILs); 01-codegen-schema 1a-1d
(schema parity Go-Java-cpp + Python output byte-equal Go-Java-cpp)
all PASS.
Refs: 2026-06-22 incident postmortem (recsys for_you), code review
.code-review/from-24975c2/from-24975c2-to-77b6e18.md.
* test(cross-validate): pin Go-Java markdown byte-equal as a 01-codegen-schema gate
Closes the dormant gap surfaced by the 2026-06-22 review:
01-codegen-schema.sh has long checked schema structure parity (1a/1b)
and Python output byte-equal parity (1c/1d), but never compared the
generated markdown across engines. Three doc-rendering drifts had
been sitting under that blind spot — see commit 75ce3c0 (the
required-first ordering + Java/Go doc fixes batch).
Adds section 1e: run pine-go and pine-java codegen with -doc-dir to a
scratch directory each, then `diff -r` the two trees. Fails the
section on any byte-level disagreement and dumps the diff to stderr
so future drifts surface in CI rather than at the next review.
Two ancillary fixes were needed to make the gate actually pass:
(1) pine-java's `if (md != null)` guard around the Metadata Contract
section let operators without a parsed javadoc Metadata block
(transform_bench_cpu / transform_bench_sleep) silently omit the
section; pine-go's template emits it unconditionally with all four
rows reduced to "-". Java now mirrors that — synthesise an empty
MetadataDoc when the parser found nothing, then render the same
four rows.
(2) pine-java's TransformCopy javadoc was missing the
"(collects all item values into a list)" annotation on the
item_to_common direction's CommonOutput row. pine-go's source
comment had it; Java's parser dropped the line, so its rendered
table cell trailed Go's by one phrase.
(3) Schema description fixup: redis_set's fail_on_error description
was the wrong wording on Java side ("logging to stderr" instead
of pine-go's "logging and continuing"). Aligned.
pine-cpp has no markdown emit path today, so the gate covers Go-Java
only. Adding the Go-cpp arm is on the follow-up: it requires lifting
metadata onto OperatorEntry (cpp has no source-comment parser) and
porting the doc template to cpp. The schema-side groundwork is in
place — types.OperatorSchema would learn a Metadata field — but the
22-operator migration belongs in its own commit so this gate can land
clean.
Test:
- cross-validate green (no real FAILs); section 1e reports
"PASS: codegen Markdown output parity Go vs Java (byte-level
match)".
- 245/245 pine-java tests still green.
- Sanity-checked the gate itself by temporarily reverting the
backtickOrDash change: the gate FAILed with a recall_resource
Metadata Contract divergence, exactly the class of regression
we want it to catch.
Refs: 2026-06-22 incident postmortem (recsys for_you), code review
.code-review/from-24975c2/from-24975c2-to-77b6e18.md.
* feat(pine-cpp): add markdown doc emit; close cross-engine doc parity
The 2026-06-22 review (.code-review/from-24975c2/...) recommended
gating Go-Java markdown byte-equality in cross-validate; the previous
commit landed that gate. Following through on the request to extend
the gate to pine-cpp ("不要后退, 不要打折扣"): pine-cpp's
pineapple-codegen now has a markdown emit path of its own, the gate
covers all three engines symmetrically, and the schema-side change
keeps metadata authoring in lockstep with how Go and Java work.
Changes:
(1) OperatorSchema gains an optional MetadataDoc field
pine-cpp/include/pine/operator.hpp adds MetadataDoc + a
schema.metadata field. Empty fields render as "-" in the generated
markdown (matches BacktickWrap on the Go side and backtickOrDash on
the Java side). pine-cpp has no source-comment parser like
pine-go's pkg/codegen/docparse.go or pine-java's
parseOperatorDocs, so cpp operators must declare metadata inline at
registration time. The cross-engine codegen contract is now
consistent: Go/Java parse from comments/javadoc, cpp declares
inline; all three feed the same renderer shape.
(2) 19 cpp operators populate .metadata to match pine-go's
source-comment metadata verbatim
filter_{condition,paginate,truncate}, merge_dedup, observe_log,
recall_{resource,static}, reorder_{shuffle_by_salt,sort},
transform_{by_lua,by_remote_pineapple,copy,dispatch,normalize,
redis_get,redis_set,resource_lookup,size}, transform_bench_{cpu,
sleep}. The strings are extracted from the rendered Go markdown
(so e.g. transform_copy's CommonOutput inherits the
"(collects all item values into a list)" annotation that Go's
metadata-section parsing collected from the multi-direction
comment block).
(3) pineapple-codegen learns a -doc-dir flag
pine-cpp/cmd/pineapple-codegen/main.cpp grows write_operator_doc_md
+ write_doc_index_md + an op_type_pascal helper that mirrors
pine-go's `string(schema.Type)` (PascalCase) and pine-java's
pascalCaseEnum. The output is byte-equal with pine-go's emit when
every operator declares matching metadata.
(4) cross-validate gate extends to Go vs cpp
scripts/cross-validate/01-codegen-schema.sh's section 1e gains a
Go-vs-cpp arm symmetric with the existing Go-vs-Java arm. The cpp
arm only runs when CPP_CODEGEN is set (i.e. the cpp binary is
built), matching the existing skip-when-absent pattern in 1d.
Test:
- 215/215 pine_cpp_tests still green.
- cross-validate green; section 1e reports both
"PASS: codegen Markdown output parity Go vs Java" and
"PASS: codegen Markdown output parity Go vs C++".
- Sanity: diff -r doc/operators against pineapple-codegen
-doc-dir output produces zero diffs.
Refs: 2026-06-22 incident postmortem (recsys for_you), code review
.code-review/from-24975c2/from-24975c2-to-77b6e18.md.
* feat(redis): per-command observability metrics (latency + outcome)
Adds two new metrics on every redis_connection resource (gated by the
existing metrics_name knob, like the pool gauges and PING probe):
pine_redis_command_duration_seconds histogram labels: name, command, status
pine_redis_command_total counter labels: name, command, status
Status taxonomy (identical across pine-go / pine-java / pine-cpp):
ok — call returned (redis.Nil cache miss is not an error)
timeout — context.DeadlineExceeded or net.Error.Timeout()
pool_timeout — connection-pool wait deadline exhausted
error — anything else (connection reset, AUTH, protocol)
Why now
-------
The 2026-06-22 recsys outage's only signals were the PING probe
(sampled every 15s) and the pool gauges. Neither distinguishes "Redis
is slow" from "we ran out of pool" from "Redis up but rejecting"; the
review (.code-review/from-24975c2/...) flagged the gap. With these
metrics the per-second timeout rate is queryable, and a Grafana panel
on rate(pine_redis_command_total{status="timeout"}[1m]) will fire
during the next incident's first 5s of deviation rather than after
the dashboard reader correlates pool usage by hand.
Implementation per engine
-------------------------
pine-go: redis.Hook on the *redis.Client wraps every command;
recordCommand classifies the error and emits both metrics. The hook
filters HELLO / CLIENT / PING / AUTH / SELECT (lifecycle + probe
traffic) so the metric reflects business commands only — pine-java
and pine-cpp instrument at the operator-level facade and never see
those, so filtering Go keeps the three engines emitting comparable
cells under the same workload (cross-validate 16 asserts cell-set
parity).
pine-java: Jedis has no hook interface, so RedisConnResource grows
runCommand(command, action). transform_redis_get / transform_redis_set
route every call through it; the pool.getResource try-with-resources
moves into the helper. redisCommandStatus walks the JedisException
cause chain (SocketTimeoutException → timeout) and recognises the
JedisException("Could not get a resource") pool-exhaustion signature.
pine-cpp: same shape as Java — RedisConnResource::run_command<T>(name,
fn) template. The cpp client throws std::runtime_error for every
failure mode, so timeout / pool_timeout aren't currently distinguished
(all non-success lands under "error"); future work is to make Client
throw distinct types so cpp can match Go's classifier.
Tests
-----
pine-go (TestRedisCommand*): classifier table covering all 7 status
cases, end-to-end miniredis fixture asserting label tuples on SET /
GET / cache-miss, gating test for empty metrics_name, filter test
proving HELLO / CLIENT / PING are elided while SET is recorded.
pine-java (RedisConnResourceTest.commandStatusClassification): pins
the JedisException -> status mapping with sentinel exceptions for
each branch (pool timeout, socket timeout, connection refused,
data error, generic).
pine-cpp (test_redis_resource.cpp kRedisMetrics): adds the two new
metric names to the gate list — both must register when metrics_name
is set, neither when it's empty.
cross-validate (section 16): metric-name list expanded to 6; per-
command cells are stripped from the byte-equal shape diff because
Java/cpp instrument at the operator boundary while go-redis surfaces
all command traffic, so the cells legitimately differ outside the
business-command intersection. Names + correctness still parity-
gated.
Operator wiring on tipsy-recsys's own redis ops
(transform_redis_zrangebyscore, transform_query_blocked_creators) is
out of scope for this PR — the upstream resource emits the metrics,
but the recsys-side operator instrumentation will land in a recsys
follow-up.
Refs: 2026-06-22 incident postmortem (recsys for_you), code review
.code-review/from-24975c2/from-24975c2-to-77b6e18.md.
* chore(redis): close two minor findings from increment-2 review
(1) pine-cpp run_command no longer prefixes the dial-failure message
with "redis: ". The pre-refactor cpp ops produced the warning
"transform_redis_get: Get(<key>): connection failed"; the refactor
accidentally drifted to "...: redis: connection failed" because
the std::runtime_error thrown from run_command embedded its own
prefix, then the operator's on_failure(e.what()) wrapped it again.
The 2026-06-23 increment-2 review (.code-review/from-24975c2/...)
flagged this as a minor wording shift not under byte-exact
cross-engine parity but worth restoring.
(2) Removed pine-java TransformRedisGet.borrowPool. The runCommand
refactor switched both internal callers (TransformRedisGet,
TransformRedisSet) to borrowResource(...) directly, leaving
borrowPool a one-line wrapper with zero callers. CLAUDE.md's
project convention is to avoid backwards-compat shims while
in-development; the same review noted it as dead code.
Test:
- 215/215 pine_cpp_tests green.
- 246/246 pine-java tests green.
- cross-validate green (no real FAILs).
Refs: 2026-06-23 review increment-2-to-4a3bf3e.md, code review
.code-review/from-24975c2/.
* docs(llmdoc): record redis cascade-safety + per-command observability + codegen parity
Reflection:
llmdoc/memory/reflections/redis-cascade-safety-and-observability.md
Stable doc updates promoted from the reflection's 5 candidates:
- must/conventions.md: codegen single-direction alignment (Go is source
of truth; pine-java / pine-cpp align to Go; do not push fields onto Go
schema to make Java align). pine-cpp raw socket clients must use
MSG_NOSIGNAL (the redis client missed it; surfaced via the AUTH-failure
test fixture).
- reference/operator-contract.md: failed-path silent-degrade audit
contract — every new client/resource failure path (AUTH/SELECT/dial/
pool) must walk the fail_on_error=false → connected()==false → silent
degrade chain end-to-end. Redis chapter's metrics_name now points at
both pool/probe and per-command metric groups.
- reference/metrics-observability.md: per-command Redis metrics
(pine_redis_command_duration_seconds + pine_redis_command_total),
status taxonomy (ok / timeout / pool_timeout / error), lifecycle-
command filtering (HELLO / CLIENT / PING / AUTH / SELECT) on the Go
hook so cells stay byte-comparable across engines whose facades sit
at different layers, and the cpp known follow-up (single
std::runtime_error type collapses timeout/pool_timeout into error).
- guides/cross-layer-validation.md: codegen markdown byte-equal gate as
the final guard (unit tests pinning a single helper aren't enough);
01-codegen-schema.sh's section 1d/1e covers Go-vs-Java + Go-vs-cpp.
- guides/standard-workflow.md: review-driven scope expansion is OK —
don't default to compromise. Per-command observability landed because
the first review round flagged it; the cleanest implementation cost
~1.5h more than the compromise variant.
- architecture/pine-cpp-runtime.md: pineapple-cpp-codegen -doc-dir
markdown emit + OperatorSchema.metadata field; redis Client AUTH/
SELECT failure containment + send(MSG_NOSIGNAL); run_command<T>
template + error-type stratification known follow-up.
- index.md: indexed the new reflection, adjusted descriptions to point
at new sections.
No quantitative numbers hard-coded (count of metrics / operators /
cross-validate sections kept symbolic); follows the project convention
on numeric-description avoidance.
* fix(pine-go): drop redundant metricRecorder selector in test recordingProvider
Pre-push lint flagged QF1008: recordingProvider embeds metricRecorder so
r.record(...) resolves the same way as r.metricRecorder.record(...) but
without the redundant selector. No behaviour change.
Caught by .githooks/pre-push (golangci-lint staticcheck).
* fix(pine-go): drop context.Canceled from timeout bucket; lands in error
The agentic-pr-review on PR #137 flagged that redisCommandStatus was
classifying context.Canceled as `timeout`, which violates
metrics-observability.md's status taxonomy. The taxonomy reserves
`timeout` for read/write_timeout_ms expirations (i.e. "Redis is
slow") so that
rate(pine_redis_command_total{status="timeout"}[1m])
reflects upstream pressure on the Redis path. context.Canceled
means the caller (HTTP handler / outer ctx) gave up — that's a
request-lifecycle event, not a Redis health signal — so a panel
querying status="timeout" should NOT include it.
Reclassifying it as `error` keeps the timeout bucket monosemantic.
Updated TestRedisCommandStatus/context_canceled to expect "error".
Inline comment pins the taxonomy decision so a future refactor
doesn't re-merge them.
Refs: 2026-06-23 PR #137 agentic-pr-review.
* chore(pine-java): align write_timeout_ms schema description with Go reference
The agentic-pr-review on PR #137 flagged that the write_timeout_ms
schema description text was drifting across the three engines: pine-go
and pine-cpp ended on "pine-go and pine-cpp honour read/write
independently." while pine-java's truncated before that sentence.
Behaviourally inert — apple_generated/resources.py is generated from
pine-go (the source of truth), so users see the Go wording verbatim.
The drift was only visible when auditing each engine's internal
schema export. Aligning the three sources avoids future audit noise
and keeps the cross-engine description registry comparable byte-for-
byte for the next time someone runs `Codegen --export-schema` against
Java or `pineapple-codegen -schema-json` against cpp.
cross-validate's section 1e markdown gate covers `doc/operators/`
only — resource schema descriptions are not part of that gate, so
this drift slipped through CI; reviewer correctly identified the gap.
Refs: 2026-06-23 PR #137 agentic-pr-review.1 parent 24975c2 commit 3fe7389
124 files changed
Lines changed: 2268 additions & 285 deletions
File tree
- apple_generated
- apple
- doc/operators
- fixtures
- benchmarks
- pipelines
- llmdoc
- architecture
- guides
- memory/reflections
- must
- reference
- pine-cpp
- cmd/pineapple-codegen
- include/pine
- operators
- filter
- merge
- observe
- recall
- reorder
- transform
- src/redis
- tests
- pine-go
- operators/transform
- pkg/codegen
- testdata
- pine-java
- examples
- src
- main/java/page/liam/pine
- operators
- test/java/page/liam/pine
- operators
- scripts
- cross-validate
Some content is hidden
Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | | - | |
| 1 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
413 | 413 | | |
414 | 414 | | |
415 | 415 | | |
| 416 | + | |
416 | 417 | | |
417 | 418 | | |
418 | | - | |
419 | 419 | | |
420 | 420 | | |
421 | 421 | | |
422 | 422 | | |
423 | 423 | | |
| 424 | + | |
424 | 425 | | |
425 | 426 | | |
426 | | - | |
427 | 427 | | |
428 | 428 | | |
429 | 429 | | |
| |||
435 | 435 | | |
436 | 436 | | |
437 | 437 | | |
| 438 | + | |
438 | 439 | | |
439 | 440 | | |
440 | | - | |
441 | 441 | | |
442 | 442 | | |
443 | 443 | | |
| |||
456 | 456 | | |
457 | 457 | | |
458 | 458 | | |
| 459 | + | |
| 460 | + | |
459 | 461 | | |
460 | 462 | | |
461 | 463 | | |
462 | 464 | | |
463 | 465 | | |
464 | | - | |
465 | 466 | | |
466 | 467 | | |
467 | 468 | | |
468 | | - | |
469 | 469 | | |
470 | 470 | | |
471 | 471 | | |
472 | 472 | | |
473 | 473 | | |
474 | 474 | | |
| 475 | + | |
| 476 | + | |
475 | 477 | | |
476 | 478 | | |
477 | 479 | | |
478 | 480 | | |
479 | 481 | | |
480 | | - | |
481 | 482 | | |
482 | 483 | | |
483 | 484 | | |
484 | | - | |
485 | 485 | | |
486 | 486 | | |
487 | 487 | | |
| |||
494 | 494 | | |
495 | 495 | | |
496 | 496 | | |
| 497 | + | |
| 498 | + | |
497 | 499 | | |
498 | 500 | | |
499 | 501 | | |
500 | | - | |
501 | 502 | | |
502 | | - | |
503 | 503 | | |
504 | 504 | | |
505 | 505 | | |
| |||
635 | 635 | | |
636 | 636 | | |
637 | 637 | | |
638 | | - | |
639 | | - | |
640 | 638 | | |
641 | 639 | | |
| 640 | + | |
| 641 | + | |
642 | 642 | | |
643 | 643 | | |
644 | 644 | | |
645 | 645 | | |
646 | 646 | | |
647 | | - | |
648 | | - | |
649 | 647 | | |
650 | 648 | | |
| 649 | + | |
| 650 | + | |
651 | 651 | | |
652 | 652 | | |
653 | 653 | | |
| |||
659 | 659 | | |
660 | 660 | | |
661 | 661 | | |
662 | | - | |
663 | | - | |
664 | 662 | | |
665 | 663 | | |
| 664 | + | |
| 665 | + | |
666 | 666 | | |
667 | 667 | | |
668 | 668 | | |
| |||
681 | 681 | | |
682 | 682 | | |
683 | 683 | | |
684 | | - | |
685 | | - | |
686 | 684 | | |
687 | 685 | | |
| 686 | + | |
| 687 | + | |
688 | 688 | | |
689 | 689 | | |
690 | 690 | | |
691 | 691 | | |
692 | 692 | | |
693 | 693 | | |
694 | | - | |
695 | | - | |
696 | 694 | | |
697 | 695 | | |
| 696 | + | |
| 697 | + | |
698 | 698 | | |
699 | 699 | | |
700 | 700 | | |
| |||
707 | 707 | | |
708 | 708 | | |
709 | 709 | | |
710 | | - | |
711 | | - | |
712 | 710 | | |
713 | 711 | | |
| 712 | + | |
| 713 | + | |
714 | 714 | | |
715 | 715 | | |
716 | 716 | | |
| |||
730 | 730 | | |
731 | 731 | | |
732 | 732 | | |
733 | | - | |
734 | 733 | | |
735 | 734 | | |
736 | 735 | | |
| 736 | + | |
737 | 737 | | |
738 | 738 | | |
739 | 739 | | |
740 | 740 | | |
741 | 741 | | |
742 | | - | |
743 | 742 | | |
744 | 743 | | |
745 | 744 | | |
| 745 | + | |
746 | 746 | | |
747 | 747 | | |
748 | 748 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
12 | 12 | | |
13 | 13 | | |
14 | 14 | | |
| 15 | + | |
15 | 16 | | |
16 | 17 | | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
17 | 22 | | |
18 | 23 | | |
19 | 24 | | |
20 | 25 | | |
21 | 26 | | |
22 | 27 | | |
23 | 28 | | |
| 29 | + | |
24 | 30 | | |
25 | 31 | | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
26 | 36 | | |
27 | 37 | | |
28 | 38 | | |
29 | 39 | | |
30 | 40 | | |
31 | 41 | | |
| 42 | + | |
32 | 43 | | |
33 | 44 | | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
34 | 49 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
8 | 8 | | |
9 | 9 | | |
10 | 10 | | |
| 11 | + | |
11 | 12 | | |
12 | 13 | | |
13 | | - | |
14 | 14 | | |
15 | 15 | | |
16 | 16 | | |
| |||
25 | 25 | | |
26 | 26 | | |
27 | 27 | | |
| 28 | + | |
28 | 29 | | |
29 | 30 | | |
30 | | - | |
31 | 31 | | |
32 | 32 | | |
33 | 33 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
8 | 8 | | |
9 | 9 | | |
10 | 10 | | |
| 11 | + | |
| 12 | + | |
11 | 13 | | |
12 | 14 | | |
13 | 15 | | |
14 | 16 | | |
15 | 17 | | |
16 | | - | |
17 | 18 | | |
18 | 19 | | |
19 | 20 | | |
20 | | - | |
21 | 21 | | |
22 | 22 | | |
23 | 23 | | |
| |||
33 | 33 | | |
34 | 34 | | |
35 | 35 | | |
| 36 | + | |
| 37 | + | |
36 | 38 | | |
37 | 39 | | |
38 | 40 | | |
39 | 41 | | |
40 | 42 | | |
41 | | - | |
42 | 43 | | |
43 | 44 | | |
44 | 45 | | |
45 | | - | |
46 | 46 | | |
47 | 47 | | |
48 | 48 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
8 | 8 | | |
9 | 9 | | |
10 | 10 | | |
11 | | - | |
12 | | - | |
13 | 11 | | |
14 | 12 | | |
| 13 | + | |
| 14 | + | |
15 | 15 | | |
16 | 16 | | |
17 | 17 | | |
| |||
26 | 26 | | |
27 | 27 | | |
28 | 28 | | |
29 | | - | |
30 | | - | |
31 | 29 | | |
32 | 30 | | |
| 31 | + | |
| 32 | + | |
33 | 33 | | |
34 | 34 | | |
35 | 35 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
8 | 8 | | |
9 | 9 | | |
10 | 10 | | |
11 | | - | |
12 | | - | |
13 | 11 | | |
14 | 12 | | |
| 13 | + | |
| 14 | + | |
15 | 15 | | |
16 | 16 | | |
17 | 17 | | |
| |||
27 | 27 | | |
28 | 28 | | |
29 | 29 | | |
30 | | - | |
31 | | - | |
32 | 30 | | |
33 | 31 | | |
| 32 | + | |
| 33 | + | |
34 | 34 | | |
35 | 35 | | |
36 | 36 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
8 | 8 | | |
9 | 9 | | |
10 | 10 | | |
11 | | - | |
12 | 11 | | |
13 | 12 | | |
14 | 13 | | |
| 14 | + | |
15 | 15 | | |
16 | 16 | | |
17 | 17 | | |
| |||
26 | 26 | | |
27 | 27 | | |
28 | 28 | | |
29 | | - | |
30 | 29 | | |
31 | 30 | | |
32 | 31 | | |
| 32 | + | |
33 | 33 | | |
34 | 34 | | |
35 | 35 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | | - | |
| 2 | + | |
3 | 3 | | |
4 | 4 | | |
5 | 5 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | | - | |
| 2 | + | |
3 | 3 | | |
4 | 4 | | |
5 | 5 | | |
| |||
0 commit comments