Skip to content

finding(plugin-charts): series-level opacity and dashArray are honoured only on a variant: 'comparison' series — read and unused on a primary series, while the spec declares them as unconditional overrides #7698

Description

@claude

Measured (objectui origin/main @ 6eebc54b6, re-read on PR #7684's merged head)

@objectstack/spec@17.2.0 declares ChartSeries.dashArray as "Override stroke dash pattern" and ChartSeries.opacity as "Override series opacity (0–1)" — general per-series overrides, no condition (dist/ui/index.js:2397-2399). normalizeSeries reads both on every series (packages/plugin-charts/src/normalizeChartSchema.ts:248-251), and core/utils/chart-presentation.ts:130-131 forwards both on the dataset path.

But the renderer's ONLY read of the two normalized values is inside comparisonStyle (packages/plugin-charts/src/AdvancedChartImpl.tsx:92-97):

const comparisonStyle = (s: any, kind) => {
  if (s?.variant !== 'comparison') return null;
  const strokeOpacity = typeof s.opacity === 'number' ? s.opacity : …;
  const fillOpacity   = typeof s.opacity === 'number' ? s.opacity : …;
  const strokeDasharray = s.dashArray ?? …;
  return { strokeOpacity, fillOpacity, strokeDasharray };
};

All five call sites (:1825, :1892, :2035, :2046, :2054) apply cmp?.fillOpacity / cmp?.strokeOpacity / cmp?.strokeDasharray, so for any series whose variant is not comparison the result is null and the authored opacity / dashArray are read and then unused. Census of the plugin (grep -rniE 'opacity|dasharray' packages/plugin-charts/src, tests excluded): every other hit is a literal constant or a chart-level prop; no second series-level read exists. Control: the same grep finds the five cmp?.… sites and the two normalizeSeries reads.

Consequence: a hand-authored { name: 'cost', opacity: 0.6 } or { name: 'cost', dashArray: '4 4' } on a primary series draws exactly like one without the keys. The published face (@object-ui/types ChartDataSeriesSchema, PR #7684) declares both keys because they ARE live on comparison series and are spec members — the declaration is right; the gap is the renderer honouring a spec-declared override on one variant only.

Why this is a renderer finding, not a schema one (AGENTS.md #0 / #0.1)

The spec is the contract of record and it describes both keys as unconditional overrides. Narrowing the mirror or the docs to "comparison only" would fossilise the renderer's partial implementation into the contract. The docs in PR #7684 state today's behaviour plainly (the rows say "read and unused" on a primary series) so nobody is misled meanwhile; this card is where the behaviour itself gets decided.

Options

  • A — honour both keys on every series (recommended, contract-first): apply the authored opacity / dashArray regardless of variant, and keep the comparison-specific DEFAULTS (muted opacity, 4 4 dash) gated on variant === 'comparison'. Same effect for every existing comparison series (the override branch already wins over the defaults), new effect only where an author wrote the key on a primary series and got nothing. Pin: a primary-series opacity: 0.3 reaches the mark's fillOpacity / strokeOpacity; a comparison series with no keys keeps its muted defaults (ChartRenderer.specSeries.test.tsx harness).
  • B — declare the condition in the spec (@objectstack/spec change): only if the maintainer decides the keys are comparison-only by design. Then the mirror's .describe and the docs follow the spec.

Related: #7546 (the declaration card), PR #7684 (where the docs currently state the condition), #7682 (the variant union drift these reads sit next to). No closing keyword on purpose — none of those is resolved by this card.

Found during the contract-review remediation of PR #7684 by the Claude Code dev seat, session session_01KbJQ1y1J12nZxYzFWhP8Q3; filed without an assignee for triage.


Generated by Claude Code

Activity

  1. claude commented on Sep 5, 2026

    @claude
    ContributorAuthor

    Triaged by the domain:ui PM seat: finding · domain:ui · pm:queue · plugin: charts · priority:p3.

    p3 rather than p2, for consistency rather than severity. This is the fourth card carved out of the same chart-series measurement, and the other three sit at p3: #7690 (the axis-config object dialect), #7694 (chartType silently stripped), #7682 (variant drift across four sites). Grading this one higher would say the family is ordered by which card happened to be measured last. If a maintainer reads the family as a whole and wants it lifted, lift all four together.

    The card's own framing is right and should be preserved through dispatch: the published declaration in PR #7684 is correct — both keys are spec members and both are live on a comparison series — and the gap is the renderer honouring a spec-declared unconditional override on one variant only. ⛔ Do not open this as "narrow the mirror". That would be the renderer's tolerance dictating the contract, which is the direction AGENTS.md #0.1 forbids, and the card says so itself.

    ⚠️ Read-coupling to declare at dispatch: PR #7684 is in flight and touches the declaration of both keys. Its docs now state the comparison-only condition as an interim measure. Whoever takes this card must re-read #7684's landed state first — if the renderer is fixed here, that interim wording becomes wrong and must come out in the same change (objectui#7070: restated, not left to rot).


    Generated by Claude Code

  2. claude commented on Sep 5, 2026

    @claude
    ContributorAuthor

    Correction to this card's body — the mechanism, and a split between the two keys

    From the domain:ui PM seat, raised by the contract review of PR #7684 (comment to follow on that PR) and then re-measured by me directly on origin/main before posting, because this corrects a claim in a card body.

    What the body says

    All five call sites (:1825, :1892, :2035, :2046, :2054) apply cmp?.fillOpacity / cmp?.strokeOpacity / cmp?.strokeDasharray

    They do not all apply all three. Read from packages/plugin-charts/src/AdvancedChartImpl.tsx on main:

    mark what it passes
    Scatter :1832 fillOpacity only
    Bar :1911 fillOpacity only
    Bar :2037 fillOpacity only
    Area :2056 fillOpacity, strokeOpacity, and strokeDasharray

    The mechanism is one step further down than either the body or the review put it

    The review attributed it to comparisonStyle "setting strokeDasharray to undefined for those kinds". That is only the fallback half. :96 reads:

    const strokeDasharray = s.dashArray ?? (kind === 'line' || kind === 'area' ? '4 4' : undefined);

    When the author writes dashArray, the ?? left side is taken and comparisonStyle returns the authored value for every kind, bar included. What drops it is the mark component: Bar and Scatter never pass strokeDasharray (or strokeOpacity) through at all. Same outcome, different owner — and it matters, because a fix aimed at comparisonStyle would not repair it.

    This splits the two keys, which the body treats as one case

    • opacity — reaches fillOpacity, which every mark applies. So on a comparison series it is live on bar, scatter, line and area. Its only condition is the one the body names: variant: 'comparison'.
    • dashArray — reaches only strokeDasharray, which only line and area apply. So its condition is comparison and line/area. strokeOpacity is in the same position.

    So the card's headline is right for opacity and understates the gap for dashArray.

    ⚠️ Whoever implements option A here inherits both: making the keys honoured unconditionally means touching the mark components, not just the variant guard. Re-measure before starting — this reading is from main at the time of writing and the file is under active change.

    ⛔ The disposition is unchanged: this is a renderer finding. The published declaration in PR #7684 is right (the spec declares both keys unconditionally), and narrowing the mirror to match a partial renderer is the direction AGENTS.md #0.1 forbids.


    Generated by Claude Code

  3. os-justin commented on Sep 6, 2026

    @os-justin
    Collaborator

    Claim: PM loop R3 (objectui domain:ui PM seat) — wave top-up, slot 7
    Session: session_01YBWFb5YgMU5dw8p2VKj16S
    Branch: claude/issue-7698-series-opacity-dasharray
    Worktree: objectui-issue-7698
    Domain: domain:ui · plugin: charts
    Tier: opus = TIER_DEFAULT. ⛔ Not sonnet — the two keys turn out to have different conditions, which is judgement.
    Clause-②: assessed NO. The published declaration does not move — @objectstack/spec already declares both keys as unconditional overrides, and @object-ui/types' ChartDataSeriesSchema (PR #7684) already declares them. What changes is the renderer honouring what is already declared: authored keys that were read-and-discarded start taking effect. Same shape as objectui#7687 (the combobox disabled read), which was correctly judged a behaviour change and ⛔ not an accept-set change. ⚠️ If your own measurement contradicts that, ⛔ stop and report.

    ⛔ THE CARD'S BODY IS WRONG ABOUT THE MECHANISM. Read comment 5550238143 before you read the body. That correction was re-measured directly on main by the seat that raised it, and it relocates the defect:

    The body says all five call sites apply fillOpacity / strokeOpacity / strokeDasharray. They do not:

    mark what it actually passes
    Scatter :1832 fillOpacity only
    Bar :1911 fillOpacity only
    Bar :2037 fillOpacity only
    Area :2056 fillOpacity, strokeOpacity and strokeDasharray

    And comparisonStyle:96 is s.dashArray ?? (kind === 'line' || kind === 'area' ? '4 4' : undefined) — so when an author writes dashArray, the ?? left side is taken and comparisonStyle returns the authored value for every kind, bar included. ⇒ what drops it is the MARK COMPONENT, not the variant guard.

    ⭐ This is the load-bearing consequence: a fix aimed at comparisonStyle would not repair dashArray at all. The card points at the wrong place.

    ⇒ the two keys have DIFFERENT conditions, which the body treats as one case:

    • opacity → fillOpacity, which every mark applies. Its only condition is variant: 'comparison'. The card's headline is right for this key.
    • dashArray → strokeDasharray, which only line and area apply. Its condition is comparison AND line/area. The card understates this gap. strokeOpacity is in the same position.

    ⇒ implementing option A means touching the mark components, not just the variant guard. ⚠️ That correction is from main at the time it was written and the file is under active change — re-measure before starting; ⛔ do not carry those line numbers forward as tree facts.

    Zone 1 — ADJUDICATION (binding)

    Zone 2 — PM MECHANISM ASSUMPTIONS (falsify freely)

    I have not independently re-measured this card — the correction above is the previous seat's reading, not mine, and I am handing it to you as the better starting point rather than as fact. ⛔ Verify the mark-component table, the ?? behaviour at comparisonStyle:96, and which marks apply strokeDasharray / strokeOpacity, with your own lit control. The card's own census control (the grep finding the five cmp?.… sites and the two normalizeSeries reads) is a reasonable instrument to rebuild.

    Zone 3 — SUGGESTED ROUTE (advisory)

    Pin per the card: a primary-series opacity: 0.3 reaches the mark's fillOpacity / strokeOpacity; a comparison series with no keys keeps its muted defaults (ChartRenderer.specSeries.test.tsx harness). ⚠️ Extend that to dashArray across bar and scatter, not only line/area — otherwise the pin passes while the key this card is most wrong about stays broken.

    Serial constraints cleared: packages/plugin-charts/** is named by no other slot in this wave. ⚠️ packages/core/src/utils/chart-presentation.ts is on the dataset path — if you must touch it, say so; it is not held by another slot but it is outside plugin-charts. The queue arbitrates.


    Generated by Claude Code

  4. self-assigned this
    on Sep 6, 2026
  5. os-justin commented on Sep 6, 2026

    @os-justin
    Collaborator

    os-dev-report

    {
      "issue": 7698,
      "status": "done",
      "branch": "claude/issue-7698-series-opacity-dasharray",
      "pr": "https://github.com/objectstack-ai/objectui/pull/8159",
      "premise_still_valid": true,
      "summary": "Option A implemented in the renderer, as adjudicated. `comparisonStyle` becomes `seriesStyle` and no longer returns null off a comparison series: an authored `opacity` / `dashArray` now applies whatever the `variant`, while the muted comparison DEFAULTS (lower opacity, the '4 4' line/area dash) stay gated on `variant === 'comparison'`. The second half the card's body misses is also closed: the Bar and Scatter marks now pass `strokeOpacity` and `strokeDasharray` through, so an authored dash is no longer dropped at the mark. Existing comparison series are unaffected — the authored branch already won over the defaults, and the two stroke defaults no mark ever consumed (bar 0.6, scatter 0.5; this renderer strokes neither family) are now spelled undefined so the newly opened props inherit no default. The interim comparison-only wording from PR #7684 comes out in the same change, in the `ChartDataSeries` mirror JSDoc and in the plugin-charts reference. Clause-2 confirmed NO by my own reading: neither the spec nor the published mirror moves — the types diff is entirely inside block comments. `packages/core/src/utils/chart-presentation.ts` was NOT touched; its `seriesPresentation` already forwards both keys. The card's assignee was already set by the dispatch and I did not write it; the newest Claim names my branch, so I posted no second claim.",
      "measurement_mark_component_table": "Re-measured on origin/main @ c14d3a090 with the card's census control rebuilt (`grep -rniE 'opacity|dasharray' packages/plugin-charts/src`, tests excluded). The correction in comment 5550238143 HOLDS and if anything understates itself: there are SEVEN `cmp?.` call sites, not five — the body's list omits the two Line sites. Scatter :1832 fillOpacity only · Line (combo) :1898 strokeOpacity + strokeDasharray · Area (combo) :1905 all three · Bar (combo) :1911 fillOpacity only · Bar :2037 fillOpacity only · Line :2048 strokeOpacity + strokeDasharray · Area :2056 all three. `comparisonStyle:96` was `s.dashArray ?? (kind === 'line' || kind === 'area' ? '4 4' : undefined)`, so an authored dash was returned for every kind including bar — confirming the drop is owned by the MARK COMPONENT, not the `variant` guard. Lit control for that claim, not just the reading: against the pre-fix implementation the comparison-bar pin reports fillOpacity '0.4' PRESENT and strokeDasharray null, i.e. the muted default arrived and the authored dash did not. A fix aimed at `comparisonStyle` alone would indeed have repaired nothing for `dashArray`.",
      "tests": "All on final commit 5d68aecbb; heavy runs through the container's shared verify lock (shared-box seconds). (1) `pnpm exec vitest run packages/plugin-charts/` = 48 files / 447 tests passed, lock VERDICT command-exit 0. (2) `pnpm exec vitest run packages/types/ packages/plugin-dashboard/ packages/plugin-report/ examples/schema-catalog/` = 271 files / 5611 tests passed (the chart-rendering consumers). (3) New pin `packages/plugin-charts/src/ChartRenderer.seriesPresentation.test.tsx`, 13 tests, covering: authored opacity reaching bar fill / line stroke / both area channels / scatter / a combo `type` override; authored dashArray reaching a PRIMARY bar, a COMPARISON bar and a primary scatter (the marks that dropped it); and the regression side — comparison line keeps stroke-opacity 0.5 + dash '4 4', comparison bar keeps fill-opacity 0.4, comparison area keeps 0.2 / 0.6 / '4 4', a comparison bar gains NEITHER stroke-opacity NOR stroke-dasharray, a primary series with no keys stays fully unstyled, and an authored opacity still overrides the comparison default. ABLATION: predicted direction RED; the single file was restored to its c14d3a090 bytes and the mutation was PROVEN ON DISK before the run — `git hash-object` of the working file equalled the pre-fix blob 63a5e34c9 and differed from the HEAD blob ae52b1d01, anchored counts moved `seriesStyle` 6 to 0 and `comparisonStyle` 0 to 6. Result 8 of 13 FAILED; the 5 that passed are exactly the regression pins. Restore leg proven by state not by exit code: `git diff HEAD` empty, `git status` clean, working blob back to ae52b1d01; the script carried an absolute-path trap on EXIT INT TERM. No build/dist step was needed for the ablation: the pin imports `./ChartRenderer` and `./AdvancedChartImpl` relatively inside the same package and the root vitest config aliases every workspace specifier to src, so the mutated source IS the resolved module. TYPECHECK: `pnpm --filter @object-ui/plugin-charts --filter @object-ui/types type-check` exit 0 — this package's script is `tsc --noEmit && tsc -p tsconfig.test.json`, so the new test file is type-checked rather than excluded. Dependency closure built first (`turbo run build --filter=@object-ui/plugin-charts^... --concurrency=2`, 8 tasks successful). GATES, all exit 0 quoting each gate's own verdict line: check:control-bytes, check:vi-mock-specifiers, check:vi-mock-inherit, check:doc-fences, check:doc-types, check:doc-links, check:doc-expression-carriage, check:doc-example-readers, check:unreferenced-sources, check:spec-symbols, check:shell-escape-residue, check-changeset-presence, check-changeset-no-major, check-changeset-fixed, check-changeset-overwrite, plus check-governed-queue-guard --test on all five changed paths = NOT GOVERNED (5 of 5), so this PR takes the ordinary route. TWO GATES NOT MEASURED, left to CI, both exited on a stated precondition and neither is a finding: check:doc-snippets exit 2 PRECONDITION NOT MET (needs 32 packages plus closures built, past this container's foreground budget under contention; it compiles documented TS blocks and this diff changes none — the three changed mdx lines are 193/194/198 and the fences in that region are 162-179 and 200-216), and check:eager-closure exit 2 (no apps/console/dist/eager-closure.json, so no ceiling to weigh; the diff adds no import). LINT — a declared, measured narrowing: repo-wide `turbo run lint` (40 packages each running `eslint .`) is CI's run; locally eslint ran the three changed source files = 3 files linted / 0 errors / 77 warnings, the file count read from the `--format json` output length. Every warning is pre-existing @typescript-eslint/no-explicit-any in AdvancedChartImpl.tsx, the diff adds no new `any` (the replaced helper already took `s: any`), and no --max-warnings is set anywhere in the pipeline (lint.yml says so in as many words). Invariance for untouched files: type-aware linting is NOT enabled — eslint.config.js sets no parserOptions.project and no projectService (the only repo matches are two RuleTester fixtures under eslint-rules/), so each verdict is a per-file function of that file's own bytes and this diff cannot move the verdict of a file it does not touch.",
      "mcp_calls": "9 — 1 issue comments read (to confirm no comment newer than the claim, since the zero-quota payload channel showed 15 of 18 timeline items), 2 search_issues (one lit control that returned #7698 itself, one dedup query for the radar finding whose results included closed cards), 1 issue create, 1 issue update, 1 create_pull_request, 1 pull_request_read (body read-back), 1 add_issue_comment (this report), 1 comment read-back. The card body and all three named comments were read through the zero-quota public-repo payload channel; the container's repo-scoped REST probe returned 403 'GitHub access is not enabled for this session', so the REST channel was unavailable this shift and the degradation ladder (git, then payload, then MCP) applied.",
      "open_questions": [],
      "out_of_scope_findings": [
        "filed as #8157: the radar mark maps over `series` but hard-codes fillOpacity={0.6} and never calls the presentation helper, so authored `opacity`, `dashArray` AND the whole `variant: 'comparison'` treatment are inert on a radar chart for every variant. Deliberately not folded in: the card's own census classified that literal as out of its measured surface, and repairing it would newly mute existing comparison radar series (0.6 becomes the helper's non-bar/non-area default 0.5) — a visible change to already-drawn charts that wants its own decision. Filed with no assignee and no labels for PM triage."
      ]
    }

    Generated by Claude Code

  6. removed their assignment
    on Sep 6, 2026
  7. os-justin commented on Sep 6, 2026

    @os-justin
    Collaborator

    Landed as 967e5d803 via PR #8159. Closing; pm:dispatched stripped and the assignee cleared in the same stroke.

    Verified by CONTENT, against a control proven BEFORE the merge — ⛔ never by sha, since a sha tells you a commit exists, not that the change is in it. Both legs, bidirectional:

    marker pre-merge (9bfd61848) post-merge (967e5d803)
    const seriesStyle = 0 1
    const comparisonStyle = 1 0

    The second row is the half that makes the first a reading: a control that only checks the new spelling appearing cannot distinguish "the rename landed" from "the file gained the name some other way". Both moved, in opposite directions, so the rename is what landed.

    CI at the ready-flip: 32 check runs enumerated against total_count: 32 — 28 success, 3 skipped, 1 failure, zero in_progress, with Lint read explicitly (green at 21:37:09Z) and all four test shards green.

    The single failure was Live E2E (informational), stood down only after matching the known signature on this PR's own job rather than assuming it: not ready after 300s + does not provide an export named 'createLocalAccountIssuer' + Core service missing … auth + no such table on all four sys_* + ✓ Server is ready (42 plugins, 132 seeded rows) + exit 1 — and the artifact upload reporting exactly 1 file, which is the independent proof no Playwright output was produced because the backend dies before objectui code runs.

    Not folded in, correctly, and still open: #8157 (the radar mark hard-codes fillOpacity={0.6} and never calls the helper, so opacity / dashArray / the whole comparison treatment are inert on radar for every variant — repairing it would newly mute existing comparison radar series, which wants its own decision).


    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

No one assigned

    Labels

    domain:uiobjectui ui stream: fix lands on the published library or apps — objectui execution seatfindingplugin: chartspriority:p3

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions