Skip to content
This repository was archived by the owner on Nov 4, 2026. It is now read-only.
This repository was archived by the owner on Nov 4, 2026. It is now read-only.

classifyAnalyticsFailure reads the status before the code, so /analytics/query's CUBE_NOT_FOUND is warned as a missing capability and silently degraded #5721

Closed

Description

@os-zhuang

Found while fixing the sibling defect on queryDataset (related to #5663, which covers POST /analytics/dataset/query only). Not fixed there: this is a different route face with a different failure mode, and correcting it changes degradation behaviour rather than copy, so it needs its own measurement and its own pins.

The defect

packages/data-objectstack/src/index.ts, classifyAnalyticsFailure:

if (code === 'VALIDATION_FAILED' || status === 400) return { kind: 'rejected', code, message };
if (status === 404 || status === 501 || code === 'NOT_IMPLEMENTED' || code === 'ROUTE_NOT_FOUND') {
  return { kind: 'not-installed', code, message };
}
return { kind: 'unknown', code, message };

The status is tested before the code and short-circuits it, so any 404 from the /analytics/query face is classified not-installed no matter what code it carries. The code operands after the || can only ever be reached by a 404-less/501-less error, i.e. they are effectively dead for the conditions they name.

Why this is not cosmetic

The consumer (same file, the aggregate catch) does two things with not-installed:

if (failure.kind === 'not-installed') {
  this.warnAnalyticsCapabilityOnce(failure.message);
}
// …then degrade to find() + client-side aggregation

So a misclassified 404 both (a) tells the operator the analytics capability is missing and (b) silently answers the chart from a different code path. That is the misdirection framework#3878 documents, arrived at from the other direction.

A measured 404 that is not a missing capability

@objectstack/service-analytics's cube-existence gate (analytics-service.ts, assertInferableCube, framework#3867):

err.code = 'CUBE_NOT_FOUND';
err.status = 404;

@objectstack/rest's dataset route comment names the same gate as CUBE_NOT_FOUND/404 in its ADR-0112 passthrough list, and packages/spec/src/api/error-code-ledger.zod.ts carries CUBE_NOT_FOUND as a catalog member. A misspelled or unregistered cube name is an authoring mistake; today it reads as "this deployment has no analytics" and the chart quietly renders numbers from find() + client-side aggregation instead.

401 UNAUTHENTICATED on this face is a second case: it lands in unknown and degrades silently, so an expired session renders a chart built from a find() that is itself about to be refused.

What the fix probably is

Mirror what landed for queryDataset: branch on the ADR-0112 code first and consult the status only as a residual when the answer declares no code at all. Two things must be established first, which is why this is not a drive-by:

  1. The envelope family differs. /analytics/query exits through @objectstack/runtime's dispatcher-plugin.errorResponseBase, i.e. the wrapped { success: false, error: { code, message } } shape, not the flat one the dataset route writes. Whether @objectstack/client's fetch wrapper flattens error.code onto the thrown error for this face needs to be measured, not assumed.
  2. The blast radius is behavioural. Every reclassification moves a chart between "degrade silently", "degrade loudly" and "throw", so each new branch needs a pin saying which of the three it gets and why.

Where to look

  • packages/data-objectstack/src/index.ts — classifyAnalyticsFailure and its single caller in the aggregate catch.
  • packages/data-objectstack/src/aggregate-capability.test.ts — the existing pins for this classifier.
  • Framework (read-only reference): packages/runtime/src/dispatcher-plugin.ts for the wrapped envelope, packages/services/service-analytics/src/analytics-service.ts for CUBE_NOT_FOUND.

Activity

  1. added
    bugSomething isn't working
    domain:uiobjectui ui stream: fix lands on the published library or apps — objectui execution seat
    on Aug 22, 2026
  2. added theissue type on Aug 24, 2026
  3. claude commented on Aug 24, 2026

    @claude
    Contributor

    Triage (daily round, session session_01Kktexqp6uVuFMztvvTMf3V, 2026-08-24): pm:queue added on existing domain:ui, Bug — classifyAnalyticsFailure reads status before code, so CUBE_NOT_FOUND degrades silently as a missing capability. Read the code first, warn loudly for producer-marked refusals.


    Generated by Claude Code

  4. self-assigned this
    on Aug 24, 2026
  5. os-warren commented on Aug 24, 2026

    @os-warren
    Collaborator

    Claim: PM loop round 34
    Session: session_019ZyKZejBWZoCSj1NP35wcp
    Branch: claude/issue-5721-analytics-failure-code-first
    Worktree: objectui-issue-5721
    Domain: domain:ui
    File surface: packages/data-objectstack/src/index.ts (classifyAnalyticsFailure + its single caller in the aggregate catch) and packages/data-objectstack/src/aggregate-capability.test.ts (+ a changeset) (stop on breach; explain in the report)
    Container & model: M, mode:subagent, model: opus
    Clause-②: no — this reclassifies client-side failure handling in a consumer package; it does not change what packages/spec accepts or rejects, and adds no published surface. ⚠️ It does change behaviour (which failures degrade silently, degrade loudly, or throw), which is why the card asks for a pin per branch.
    Serial constraints cleared: packages/data-objectstack/** — no in-flight claim in this lane touches it; zero claude/issue-* branches on the remote. Last change to src/index.ts was d40d295 (2026-08-23, citation re-anchor only).

    Two things the card asks you to establish by measurement before writing the fix — both are genuine open questions, not rhetorical:

    1. /analytics/query exits through the wrapped { success: false, error: { code, message } } envelope, unlike the flat one queryDataset writes. Whether @objectstack/client's fetch wrapper flattens error.code onto the thrown error for this face is unmeasured — measure it; ⛔ do not assume it matches the dataset route.
    2. Every reclassification moves a chart between degrade silently, degrade loudly and throw. Each new branch owes a pin naming which of the three it gets and why.

    ⛔ If the only way to make the classifier work is to widen a type or loosen an existing pin, stop and report rather than doing it. Framework paths under packages/runtime and packages/services are read-only reference here — they are another repo's lane.


    Generated by Claude Code

  6. os-warren commented on Aug 24, 2026

    @os-warren
    Collaborator
    {
      "issue": 5721,
      "status": "done",
      "branch": "claude/issue-5721-analytics-failure-code-first",
      "pr": "https://github.com/objectstack-ai/objectui/pull/5952",
      "premise_still_valid": true,
      "summary": "classifyAnalyticsFailure now branches on the ADR-0112 code and consults the status only as a residual (when the answer declared no code at all), so the 404 short-circuit that made NOT_IMPLEMENTED/ROUTE_NOT_FOUND unreachable is gone. Both PM assumptions were measured before any code was written. (1) The envelope family DOES differ as the card said — /analytics/query answers wrapped via runtime's dispatcher-plugin.errorResponseBase, its 401 answers flat via enforceAuth's ANONYMOUS_DENY_BODY — but the question the card left open resolves in the fix's favour: @objectstack/client's fetch wrapper flattens BOTH (`errorBody?.code ?? errorBody?.error?.code` onto error.code, plus error.httpStatus), so no envelope reading belongs in this function and it reads one field. Chain confirmed end to end, including that CUBE_NOT_FOUND is a ledger-registered code and so survives errorResponseBase verbatim rather than being demoted to declaredCode. (2) Two branches change behaviour, each with a pin naming its outcome: 404 CUBE_NOT_FOUND was degrade-loudly-with-the-wrong-message and is now THROW (rethrown verbatim, keeping the producer's code and repair instructions); 401 UNAUTHENTICATED was degrade-silently and is now THROW (reusing the existing AnalyticsUnauthenticatedError). Neither adds a published error type and no existing outcome moved. Both throws are justified by measurement rather than taste: degrading is provably futile for both — CUBE_NOT_FOUND is thrown only when the name is neither a registered cube nor a registered object, and the fallback re-reads that same name through /data where objectql's assertObjectRegistered answers 404 OBJECT_NOT_FOUND; the 401 fallback carries the same lapsed token. One refinement to the card's wording, not a falsification: for CUBE_NOT_FOUND the chart does not in fact 'quietly render numbers from find()' — the fallback cannot answer that name either, so the real pre-fix harm was a wrong operator instruction plus a confusing distant error, and the silent-second-answer harm is exact for the 401 case. Scope held: only classifyAnalyticsFailure, its single caller in the aggregate catch, that caller's test file, and one changeset. No type widened, no pin loosened, no framework path edited.",
      "tests": "All at final HEAD 8d8b0eb96 (= PR head sha), re-run after the last commit. Dependency closure built first: `pnpm --workspace-concurrency=2 --filter '@object-ui/data-objectstack^...' build` -> exit 0 (types, core). (1) `pnpm exec vitest run packages/data-objectstack/ --maxWorkers=2` -> exit 0, 'Test Files 42 passed (42)' / 'Tests 581 passed (581)'; the target file alone is 12/12 (7 pre-existing + 5 added). (2) `pnpm --filter @object-ui/data-objectstack type-check` -> exit 0, script name echoed ('> tsc --noEmit') so it was not a zero-match silent pass. (3) `pnpm exec eslint packages/data-objectstack` -> exit 0; --format json measured the population at 48 files, 0 errors, and 0 lint messages landing on added lines in index.ts (2 no-explicit-any WARNINGS on added test lines, matching the 5 pre-existing `(r: any)` usages in the same file). (4) Gate family: check:control-bytes '✅ OK (scanned 4940 tracked text file(s))', check-changeset-presence '✅ … declares 1 changeset(s)', check-changeset-no-major '✅ No changeset declares a `major` bump.', check:phantom-deps / check:self-import / check:esm-specifiers each ✅ — quoting each gate's own verdict line, and every exit code captured before any pipe. ABLATION (reverse verification, run from the COMMITTED fix so restore was a checkout, with `trap '<restore>' EXIT INT TERM`): no rebuild leg applies and none was skipped — the test imports './index' relatively inside the same package, so vitest transforms src and no dist/exports hop is in the resolution path; the dependency closure was already built above. Mutation confirmed on disk BEFORE reading any result, anchored on the text being changed rather than a bare diff: injected marker grep -c = 1, removed branch `errorCodeIs({ code }, 'CUBE_NOT_FOUND')` grep -c = 0, removed residual `status === 401` limb grep -c = 0. Direction predicted before running, then measured: 'Tests 3 failed | 9 passed (12)' — the CUBE_NOT_FOUND pin, the coded-401 pin and the code-less-401 pin all red, while BOTH controls stayed green (404 ROUTE_NOT_FOUND still degrades loudly at the same status; 500 ANALYTICS_QUERY_FAILED still degrades silently), which is what shows the new classifier did not simply turn every named failure into a throw. Restore leg verified: 0 ablation markers on disk, `git status --porcelain` empty (byte-identical to the commit), 12/12 green again. Reported honestly: a FIRST ablation attempt was partial — it replaced the code-first branches but left the new residual intact — and produced 2 red, not the 3 I had predicted; the code-less-401 pin lands in that residual. The prediction was wrong about my own mutation's extent, not about the pin, and the full ablation above establishes all three.",
      "open_questions": [],
      "out_of_scope_findings": [
        "filed as #5954: aggregate-capability.test.ts's header claims @objectstack/client's analytics.query() never throws on a non-2xx, but the installed client's fetch wrapper does throw — and every test in the file, old and new, depends on that throw"
      ]
    }

    Generated by Claude Code

  7. os-warren commented on Aug 24, 2026

    @os-warren
    Collaborator

    os-dev-report

    (Supersedes comment 5390741070 — the HTML-comment marker on that one was eaten by the body sanitizer after it was stored, so it is invisible to a marker scan. Same JSON, plain-text marker. Read this one.)

    {
      "issue": 5721,
      "status": "done",
      "branch": "claude/issue-5721-analytics-failure-code-first",
      "pr": "https://github.com/objectstack-ai/objectui/pull/5952",
      "premise_still_valid": true,
      "summary": "classifyAnalyticsFailure now branches on the ADR-0112 code and consults the status only as a residual (when the answer declared no code at all), so the 404 short-circuit that made NOT_IMPLEMENTED/ROUTE_NOT_FOUND unreachable is gone. Both PM assumptions were measured before any code was written. (1) The envelope family DOES differ as the card said — /analytics/query answers wrapped via runtime's dispatcher-plugin.errorResponseBase, its 401 answers flat via enforceAuth's ANONYMOUS_DENY_BODY — but the question the card left open resolves in the fix's favour: @objectstack/client's fetch wrapper flattens BOTH (errorBody?.code ?? errorBody?.error?.code onto error.code, plus error.httpStatus), so no envelope reading belongs in this function and it reads one field. Chain confirmed end to end, including that CUBE_NOT_FOUND is a ledger-registered code and so survives errorResponseBase verbatim rather than being demoted to declaredCode. (2) Two branches change behaviour, each with a pin naming its outcome: 404 CUBE_NOT_FOUND was degrade-loudly-with-the-wrong-message and is now THROW (rethrown verbatim, keeping the producer's code and repair instructions); 401 UNAUTHENTICATED was degrade-silently and is now THROW (reusing the existing AnalyticsUnauthenticatedError). Neither adds a published error type and no existing outcome moved. Both throws are justified by measurement rather than taste: degrading is provably futile for both — CUBE_NOT_FOUND is thrown only when the name is neither a registered cube nor a registered object, and the fallback re-reads that same name through /data where objectql's assertObjectRegistered answers 404 OBJECT_NOT_FOUND; the 401 fallback carries the same lapsed token. One refinement to the card's wording, not a falsification: for CUBE_NOT_FOUND the chart does not in fact 'quietly render numbers from find()' — the fallback cannot answer that name either, so the real pre-fix harm was a wrong operator instruction plus a confusing distant error, and the silent-second-answer harm is exact for the 401 case. Scope held: only classifyAnalyticsFailure, its single caller in the aggregate catch, that caller's test file, and one changeset. No type widened, no pin loosened, no framework path edited.",
      "tests": "All at final HEAD 8d8b0eb96 (= PR head sha), re-run after the last commit. Dependency closure built first: pnpm --workspace-concurrency=2 --filter '@object-ui/data-objectstack^...' build -> exit 0 (types, core). (1) pnpm exec vitest run packages/data-objectstack/ --maxWorkers=2 -> exit 0, 'Test Files 42 passed (42)' / 'Tests 581 passed (581)'; the target file alone is 12/12 (7 pre-existing + 5 added). (2) pnpm --filter @object-ui/data-objectstack type-check -> exit 0, script name echoed ('> tsc --noEmit') so it was not a zero-match silent pass. (3) pnpm exec eslint packages/data-objectstack -> exit 0; --format json measured the population at 48 files, 0 errors, and 0 lint messages landing on added lines in index.ts (2 no-explicit-any WARNINGS on added test lines, matching the 5 pre-existing (r: any) usages in the same file). (4) Gate family: check:control-bytes 'OK (scanned 4940 tracked text file(s))', check-changeset-presence 'declares 1 changeset(s)', check-changeset-no-major 'No changeset declares a major bump.', check:phantom-deps / check:self-import / check:esm-specifiers each green — quoting each gate's own verdict line, and every exit code captured before any pipe. ABLATION (reverse verification, run from the COMMITTED fix so restore was a checkout, with a trap on EXIT INT TERM restoring from a pre-mutation copy): no rebuild leg applies and none was skipped — the test imports './index' relatively inside the same package, so vitest transforms src and no dist/exports hop is in the resolution path; the dependency closure was already built above. Mutation confirmed on disk BEFORE reading any result, anchored on the text being changed rather than a bare diff: injected marker grep -c = 1, removed branch errorCodeIs({ code }, 'CUBE_NOT_FOUND') grep -c = 0, removed residual status === 401 limb grep -c = 0. Direction predicted before running, then measured: 'Tests 3 failed | 9 passed (12)' — the CUBE_NOT_FOUND pin, the coded-401 pin and the code-less-401 pin all red, while BOTH controls stayed green (404 ROUTE_NOT_FOUND still degrades loudly at the same status; 500 ANALYTICS_QUERY_FAILED still degrades silently), which is what shows the new classifier did not simply turn every named failure into a throw. Restore leg verified: 0 ablation markers on disk, git status --porcelain empty (byte-identical to the commit), 12/12 green again. Reported honestly: a FIRST ablation attempt was partial — it replaced the code-first branches but left the new residual intact — and produced 2 red, not the 3 I had predicted; the code-less-401 pin lands in that residual. The prediction was wrong about my own mutation's extent, not about the pin, and the full ablation above establishes all three.",
      "open_questions": [],
      "out_of_scope_findings": [
        "filed as #5954: aggregate-capability.test.ts's header claims @objectstack/client's analytics.query() never throws on a non-2xx, but the installed client's fetch wrapper does throw — and every test in the file, old and new, depends on that throw"
      ]
    }

    Generated by Claude Code

  8. os-warren commented on Aug 24, 2026

    @os-warren
    Collaborator

    ACCEPT — PR #5952, pending CI convergence

    Reviewed against GitHub, not against the report's self-account. Landing waits on the gate read; the report legitimately arrives at draft-PR time and CI convergence is this seat's to read.

    Checklist

    item reading
    PR form draft ✓, base main ✓, first line Fixes #5721 ✓
    Scope vs. declared fence 3 files, exactly the fence: packages/data-objectstack/src/index.ts, …/aggregate-capability.test.ts, one changeset. No breach.
    Changeset present, minor for @object-ui/data-objectstack, and it describes the behaviour change rather than the edit
    Framework paths untouched — read as reference only, as briefed
    CI mergeable_state: unstable, checks still running at review time — not yet read; see below

    Spot-checks — I verified the load-bearing claims myself rather than relaying them

    The report's strongest claim is "adds no published error type / no new exported surface." That is the kind of claim worth checking, and it holds on origin/main:

    • errorCodeIs / errorCodeIsAnyOf — pre-exist at packages/types/src/error-code.ts:22,29, and were already imported at packages/data-objectstack/src/index.ts:45 (three existing call sites). No new dependency.
    • ANALYTICS_ABSENT_STATUSES — pre-exists at index.ts:768.
    • AnalyticsUnauthenticatedError — pre-exists at index.ts:677. The 401 branch genuinely reuses it.

    So the new kind union members (unauthenticated, cube-not-found) are internal to the classifier's return type; nothing new leaves the package.

    What makes this a good run rather than merely a green one

    • Both PM assumptions were measured before code was written, and the envelope question — the one I flagged as genuinely open — was resolved end to end: the client's fetch wrapper flattens both families (errorBody?.code ?? errorBody?.error?.code), so no envelope reading belongs in this function at all. The tests drive the real client through a fetchImpl, so a client that stopped flattening turns them red instead of passing on a rewritten mock. That is the difference between a pin and a mock that agrees with itself.
    • The ablation has controls, and the controls are the point. Restoring the pre-fix decision body turns exactly the three new pins red while 404 ROUTE_NOT_FOUND (same status, different code) and 500 ANALYTICS_QUERY_FAILED stay green — which is what shows the code-first classifier did not simply turn every named failure into a throw. An ablation without that guard would not have distinguished the fix from a blunt one.
    • A failed prediction was reported rather than buried. A first, partial ablation produced 2 red where 3 were predicted; the dev diagnosed its own mutation as incomplete (the code-less-401 pin lands in the residual it had not touched) instead of quietly re-running until the number matched. Reporting that is worth more than the tidy result.
    • The card's wording was corrected on evidence. The card says a misclassified CUBE_NOT_FOUND makes the chart "quietly render numbers from find()". It cannot — the fallback re-reads the same name through /data and gets 404 OBJECT_NOT_FOUND. So the real pre-fix harm was a wrong operator instruction plus a distant error, and the silent-second-answer harm is exact only for the 401 case. Refining the card on measurement is the behaviour this seat wants.

    One caveat recorded, not blocking

    The 401 → throw reclassification rests on "the fallback's find() carries the same lapsed token and is refused the same way." For CUBE_NOT_FOUND the equivalent futility argument was traced to a named producer (assertObjectRegistered, 404 OBJECT_NOT_FOUND); for the 401 it is reasoned rather than traced. It is very likely right, and the prior behaviour — silently rendering a chart from a find() about to be refused — was indefensible either way, so this does not hold the PR.

    ⚠️ Flagged for whoever touches guest/preview boots next (cf. #5828): a 401 on this face now throws where it previously degraded silently. If any anonymous or preview surface renders analytics and relied on that silent degrade, this is the change that will surface it — loudly, which is the intent, but it is a behaviour change with a blast radius outside this file.

    Follow-up filed correctly

    #5954 — the test file's header claims analytics.query() never throws on a non-2xx, while every test in it (old and new) depends on the throw. Filed unassigned with finding + domain:ui, back-linked, and ⛔ not self-graded into pm:queue — first-touch grading is triage's. Correct handling.


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

bugSomething isn't workingdomain:uiobjectui ui stream: fix lands on the published library or apps — objectui execution seatpm:dispatched

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions