scrum lift_001: 4 fixes from cross-lineage review
Cross-lineage scrum on b2e45f7 produced 1 convergent + 3 single-reviewer
findings worth fixing. All apply.
1. (Opus WARN + Qwen INFO convergent) scripts/playbook_lift.sh: replace
sleep 2.5 in SQL probe with active polling up to 5s. refresh_every=1s
is a lower bound; under load the manifest may not be visible in a
fixed sleep, which would 4xx the probe and abort the reality run.
2. (Opus INFO) scripts/playbook_lift.sh: report template glued
"env JUDGE_MODEL" + value as "env JUDGE_MODELqwen2.5:latest" with no
separator. Replaced two :+/:- substitution chains with a single
JUDGE_SOURCE variable computed once at the top of the harness.
3. (Opus INFO) scripts/staffing_workers/main.go: -id-prefix "" silently
allowed, defeating the flag's purpose (cross-corpus collision prevent).
Now log.Fatal at startup with explicit hint.
4. (Opus WARN) cmd/{pathwayd,observerd}/main_test.go: newTestRouter
returned http.Handler then re-cast to chi.Router for chi.Walk.
Returning chi.Router directly satisfies http.Handler AND avoids an
assertion that would panic if future middleware wraps the router.
Dismissed (with rationale):
- Kimi INFO hardcoded MinIO endpoint: harness is local-by-design.
- Kimi WARN matrixd accepts 502/500: documented; real retriever needs
real upstreams the test doesn't spin up.
- Qwen INFO queryd string.Contains: brittle but very low risk; restating
through typed-error path would couple without adding signal.
go test ./cmd/{matrixd,queryd,pathwayd,observerd} all green.
Verdicts at reports/scrum/_evidence/2026-04-30/verdicts/lift_001_*.md.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
b2e45f7f26
commit
6c02c905c8
@ -15,7 +15,11 @@ import (
|
||||
// newTestRouter builds the observerd router with an in-memory store
|
||||
// and a workflow runner with no modes registered. Closes R-005 for
|
||||
// observerd.
|
||||
func newTestRouter(t *testing.T) http.Handler {
|
||||
//
|
||||
// Returns chi.Router (not http.Handler) so chi.Walk works without a
|
||||
// type assertion that would panic if a future refactor wraps the
|
||||
// router in plain net/http middleware.
|
||||
func newTestRouter(t *testing.T) chi.Router {
|
||||
t.Helper()
|
||||
h := &handlers{
|
||||
store: observer.NewStore(nil),
|
||||
@ -34,8 +38,7 @@ func TestRoutesMounted(t *testing.T) {
|
||||
"POST /observer/workflow/run": false,
|
||||
"GET /observer/workflow/modes": false,
|
||||
}
|
||||
router := r.(chi.Router)
|
||||
_ = chi.Walk(router, func(method, route string, _ http.Handler, _ ...func(http.Handler) http.Handler) error {
|
||||
_ = chi.Walk(r, func(method, route string, _ http.Handler, _ ...func(http.Handler) http.Handler) error {
|
||||
key := method + " " + route
|
||||
if _, ok := want[key]; ok {
|
||||
want[key] = true
|
||||
|
||||
@ -14,7 +14,11 @@ import (
|
||||
// newTestRouter builds the pathwayd router with an in-memory store
|
||||
// (nil persistor). Closes R-005 for pathwayd: 9 routes mounted with
|
||||
// no router-level test prior to this file.
|
||||
func newTestRouter(t *testing.T) http.Handler {
|
||||
//
|
||||
// Returns chi.Router (not http.Handler) so chi.Walk works without a
|
||||
// type assertion that would panic if a future refactor wraps the
|
||||
// router in plain net/http middleware.
|
||||
func newTestRouter(t *testing.T) chi.Router {
|
||||
t.Helper()
|
||||
h := &handlers{store: pathway.NewStore(nil)}
|
||||
r := chi.NewRouter()
|
||||
@ -36,8 +40,7 @@ func TestRoutesMounted(t *testing.T) {
|
||||
"GET /pathway/stats": "",
|
||||
}
|
||||
got := map[string]bool{}
|
||||
router := r.(chi.Router)
|
||||
_ = chi.Walk(router, func(method, route string, _ http.Handler, _ ...func(http.Handler) http.Handler) error {
|
||||
_ = chi.Walk(r, func(method, route string, _ http.Handler, _ ...func(http.Handler) http.Handler) error {
|
||||
got[method+" "+route] = true
|
||||
return nil
|
||||
})
|
||||
|
||||
@ -68,7 +68,16 @@ if ! curl -sS http://localhost:11434/api/tags | jq -e --arg m "$EFFECTIVE_JUDGE"
|
||||
echo "[lift] judge model '$EFFECTIVE_JUDGE' not loaded in Ollama — pull it first"
|
||||
exit 1
|
||||
fi
|
||||
echo "[lift] judge resolved to: $EFFECTIVE_JUDGE (from ${JUDGE_MODEL:+env}${JUDGE_MODEL:-config})"
|
||||
# Compute a single string for "where did the judge come from" so the
|
||||
# log line + the markdown report don't have to chain :+/:- substitutions
|
||||
# (those silently fuse "env JUDGE_MODEL" + the value into "env JUDGE_MODELx"
|
||||
# without a separator — the bug Opus caught on lift_001's report).
|
||||
if [ -n "$JUDGE_MODEL" ]; then
|
||||
JUDGE_SOURCE="env JUDGE_MODEL=${JUDGE_MODEL}"
|
||||
else
|
||||
JUDGE_SOURCE="config [models].local_judge"
|
||||
fi
|
||||
echo "[lift] judge resolved to: $EFFECTIVE_JUDGE (from $JUDGE_SOURCE)"
|
||||
|
||||
echo "[lift] building binaries..."
|
||||
go build -o bin/ ./cmd/storaged ./cmd/catalogd ./cmd/ingestd ./cmd/queryd \
|
||||
@ -207,16 +216,24 @@ id,name,role
|
||||
CSVEOF
|
||||
INGEST_RESP="$(curl -sS -F "file=@$PROBE_CSV" "http://127.0.0.1:3110/v1/ingest?name=lift_sql_probe")"
|
||||
echo "[lift] ingest response: $INGEST_RESP"
|
||||
# queryd refresh_every=1s — give it a couple ticks to discover the new manifest.
|
||||
sleep 2.5
|
||||
SQL_RESP="$(curl -sS -X POST http://127.0.0.1:3110/v1/sql \
|
||||
-H 'content-type: application/json' \
|
||||
-d '{"sql":"SELECT COUNT(*) FROM lift_sql_probe"}')"
|
||||
PROBE_COUNT="$(echo "$SQL_RESP" | jq -r '.rows[0][0] // "ERR"' 2>/dev/null || echo "ERR")"
|
||||
# Poll up to 5s for queryd to discover the manifest. refresh_every=1s
|
||||
# is a lower bound; under load or slow disks the manifest may not be
|
||||
# visible in a fixed sleep, which would 4xx the SQL probe spuriously.
|
||||
PROBE_COUNT=ERR
|
||||
SQL_RESP=""
|
||||
deadline=$(($(date +%s) + 5))
|
||||
while [ "$(date +%s)" -lt "$deadline" ]; do
|
||||
SQL_RESP="$(curl -sS -X POST http://127.0.0.1:3110/v1/sql \
|
||||
-H 'content-type: application/json' \
|
||||
-d '{"sql":"SELECT COUNT(*) FROM lift_sql_probe"}')"
|
||||
PROBE_COUNT="$(echo "$SQL_RESP" | jq -r '.rows[0][0] // "ERR"' 2>/dev/null || echo "ERR")"
|
||||
[ "$PROBE_COUNT" = "3" ] && break
|
||||
sleep 0.25
|
||||
done
|
||||
if [ "$PROBE_COUNT" = "3" ]; then
|
||||
echo "[lift] ✓ SQL surface probe passed (rowcount=3)"
|
||||
else
|
||||
echo "[lift] ✗ SQL surface probe FAILED (got: $SQL_RESP)"
|
||||
echo "[lift] ✗ SQL surface probe FAILED after 5s (got: $SQL_RESP)"
|
||||
exit 1
|
||||
fi
|
||||
|
||||
@ -275,7 +292,7 @@ generate_md() {
|
||||
# Playbook-Lift Reality Test — Run ${RUN_ID}
|
||||
|
||||
**Generated:** ${gen_at}
|
||||
**Judge:** \`${EFFECTIVE_JUDGE}\` (Ollama, resolved from ${JUDGE_MODEL:+env JUDGE_MODEL}${JUDGE_MODEL:-config [models].local_judge})
|
||||
**Judge:** \`${EFFECTIVE_JUDGE}\` (Ollama, resolved from ${JUDGE_SOURCE})
|
||||
**Corpora:** \`${CORPORA}\`
|
||||
**Workers limit:** ${WORKERS_LIMIT}
|
||||
**Queries:** \`${QUERIES_FILE}\` (${total} executed)
|
||||
@ -339,7 +356,7 @@ MDEOF
|
||||
results land in one corpus, the matrix layer's purpose isn't being tested.
|
||||
Check per-corpus distribution in the JSON.
|
||||
5. **Judge resolution.** This run used \`${EFFECTIVE_JUDGE}\` from
|
||||
${JUDGE_MODEL:+env JUDGE_MODEL override}${JUDGE_MODEL:-the lakehouse.toml [models].local_judge tier}.
|
||||
${JUDGE_SOURCE}.
|
||||
Bumping the judge for run #N+1 means editing one line in lakehouse.toml.
|
||||
|
||||
## Next moves
|
||||
|
||||
@ -274,6 +274,12 @@ func main() {
|
||||
)
|
||||
flag.Parse()
|
||||
|
||||
// An empty prefix collides cross-corpus — exactly the bug the
|
||||
// flag exists to prevent. Force callers to be explicit.
|
||||
if *idPrefix == "" {
|
||||
log.Fatalf("--id-prefix cannot be empty (use 'w-', 'e-', etc. — IDs collide cross-corpus without one)")
|
||||
}
|
||||
|
||||
hc := &http.Client{Timeout: 5 * time.Minute}
|
||||
ctx := context.Background()
|
||||
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user