Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
98 changes: 95 additions & 3 deletions src/views/task.view.ts
Original file line number Diff line number Diff line change
Expand Up @@ -286,9 +286,9 @@ export const TaskViews = defineView({
},

/**
* The same rows a manager already reads, bucketed by team. `business_unit`
* is denormalised onto the task at dispatch precisely so a rollup like
* this survives a later transfer.
* Outstanding work, bucketed by team. `business_unit` is denormalised onto
* the task at dispatch precisely so a rollup like this survives a later
* transfer.
*
* Groups sort by LABEL — measured in the grid's grouping hook, which sorts
* group keys with a locale compare on the label and never on the bucket
Expand All @@ -298,6 +298,41 @@ export const TaskViews = defineView({
label: 'By business unit',
type: 'grid',
data,
/**
* The residual, stated for the reader of the SCREEN and not only for the
* reader of this file, because the filter below does NOT make the
* mechanism correct — it makes this deployment fit inside it. The
* grouping and its per-group counts are computed over the FETCHED PAGE,
* so the numbers in the group headers are true only while the filtered
* set fits in one. The dashboard's by-unit figures come from the
* `duly_stagnation` dataset and are aggregated server-side over the
* whole store; those are the authoritative ones, and this lens is for
* browsing.
*
* ⚠ It does NOT reach the screen today, and that is measured rather
* than assumed. The value survives every layer — it is in
* `dist/objectstack.json` and in `GET /api/v1/meta/view/duly_task` —
* but the console's `ObjectView` relay builds its ListView schema by
* spreading the OBJECT's list view and relaying `label`, `sort`,
* `filter` and friends off the active view, and `description` is not
* one of the keys it relays. `ListView` has the branch that would
* render it (`data-testid="view-description"`); the value never
* arrives, so nothing is displayed and nothing errors. Filed as
* objectstack-ai/objectui#7199.
*
* It stays authored anyway, and that is deliberate: this is the key the
* spec defines for exactly this sentence, it IS served to API and MCP
* callers reading the view today, and it starts rendering the moment
* the relay is fixed. What must not happen is someone reading this file
* and believing the caveat is currently in front of users. Written as a
* PLAIN STRING on purpose: the renderer takes
* `typeof description === 'string' ? description : ''`, so an inline
* `{ en, 'zh-CN' }` locale map would render empty even after the relay
* is fixed (second half of #7199).
*/
description:
'Open and in-progress work only. Group counts are computed over the loaded page — the '
+ 'dashboard is the authoritative by-unit surface; this lens is for browsing.',
/**
* `business_unit` is in the columns because the grid's query
* projection is built from `columns` ALONE — `grouping` contributes
Expand All @@ -317,6 +352,63 @@ export const TaskViews = defineView({
*/
columns: [...columns, { field: 'business_unit' }],
grouping: { fields: [{ field: 'business_unit' }] },
/**
* ⚠ LOAD-BEARING FOR THE GROUPING — not a scope preference, and not a
* default worth inheriting. Widen it and the lens silently goes wrong
* again, in the way described below. `test/metadata-bindings.test.ts`
* fails if it is dropped or widened.
*
* ── What it is holding up, measured on the #75 seed ──────────────────
* The grid groups CLIENT-SIDE over the rows already fetched, and its
* per-group counts are `computeAggregations` over that same array
* (objectui's `useGroupedData`). There is no server-side grouping path
* for a grid at all, so a grouped grid can only be a complete roll-up
* while its whole result set fits in one page.
*
* Unfiltered, this lens did not. The store holds 186 tasks — 151 of them
* `done`, most from months ago — across FIVE business units, and the
* request is `top=100`:
*
* Northgate Operations 33 store: 61
* Northgate Plant 3 store: 7
* Northgate Quality 46 store: 86
* Riverside Plant 18 store: 31
* (Central Office) — store: 1 ← no group at all
* ───
* 100 "Showing first 100 records."
*
* Two failures, and the second is the sharper one. Every count in the
* header was a page slice reading as a total. And one unit had NO GROUP
* ON THE SCREEN, with nothing saying a unit was missing — a wrong number
* invites a second look, an absent row does not.
*
* Scoped to open work the lens fits: 27 `open` + 6 `in_progress` = 33
* rows, one page, all five units present, every count true. That is also
* the better lens on its own merits — "what is outstanding, by unit" is
* the question a manager opens this for, and nobody wants a by-unit
* breakdown of work that finished six months ago. It survives the
* platform being fixed: even with server-side grouping we would not want
* this lens spending its first page on completed tasks.
*
* ── Why not simply raise the page size ──────────────────────────────
* It moves the cliff instead of removing it, and hides the next
* occurrence. With a filter that fits, the page size is not what is
* holding the lens together.
*
* ── What stays broken ───────────────────────────────────────────────
* This remains STRUCTURALLY page-scoped. A deployment with more open
* tasks than a page hits exactly this again, with the same silent
* missing group. The filter buys a correct lens at this product's
* realistic scale, not a correct mechanism. The durable fix is upstream:
* objectstack-ai/objectui#7189 asks for server-side grouping and true
* per-group counts on a grid — the platform already does this for the
* dashboard through a dataset, just not on this surface. The `description`
* above is where that is said to users — subject to objectui#7199,
* which currently keeps a per-view description off the screen entirely,
* so today this comment and the counts' own arithmetic are the whole
* disclosure.
*/
filter: [{ field: 'status', operator: 'in', value: ['open', 'in_progress'] }],
bulkActionDefs: bulkActions,
sort: [{ field: 'due_date', order: 'asc' }],
},
Expand Down
204 changes: 204 additions & 0 deletions test/metadata-bindings.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1418,3 +1418,207 @@ describe('grouping-projection guard — the guard can fail (self-test on synthet
expect(walk.exempt).toEqual([]);
});
});

// ─────────────────────────────────────────────────────────────────────────
// The by-unit lens stays scoped to open work (stopgap for objectui#7189)
// ─────────────────────────────────────────────────────────────────────────

/**
* ⚠️ THIRD STOPGAP, and it is the SAME defect family as the grouping-
* projection guard directly above — deliberately in this file, next to it,
* rather than as a second mechanism somewhere else. That one makes the
* grouping field arrive; this one keeps the grouped set small enough for the
* grouping to be COMPLETE. Both go when **objectstack-ai/objectui#7189**
* lands, and not before.
*
* ── The defect, measured on `0f0ec49` with the #75 seed ─────────────────
* `/_console/apps/ai.objectstack.duly/duly_task/view/by_unit` carried no
* filter, so it rendered all 186 tasks — 151 of them `done`. The request is
* `top=100`, the grid groups CLIENT-SIDE over the rows already fetched, and
* its per-group counts are `computeAggregations` over that same array
* (objectui's `useGroupedData`). There is no server-side grouping path for a
* grid at all. What the screen showed:
*
* | group header | shown | in store |
* |----------------------|-------|-----------|
* | Northgate Operations | 33 | 61 |
* | Northgate Plant | 3 | 7 |
* | Northgate Quality | 46 | 86 |
* | Riverside Plant | 18 | 31 |
* | Central Office | — no group at all — | 1 |
*
* Every count was a page slice reading as a total, and one of the five units
* was ABSENT with nothing on screen saying so. The absent unit is the sharper
* half: a wrong number invites a second look, a missing row does not.
*
* Scoped to `status in ('open','in_progress')` the lens fits in one page:
* 27 `open` + 6 `in_progress` = 33 rows, all five units present, every count
* true. Counted off the seed, not off the screen.
*
* ── Why this is a PIN on one lens, not "grouped grids must be filtered" ──
* A filter requirement over every grouped grid would fire on `duly_duty ›
* catalog_tree`, which groups two levels deep and carries no filter — and
* measurably does not need one: the seed holds **31** duties, one page, all
* groups present, all counts true. Making it declare a filter to satisfy a
* rule is the same mistake the guard above narrows away from — a rule that
* fires on a non-bug teaches the next reader that the test lies. What is
* actually being guarded is not "has a filter", it is a DECISION about one
* lens, so this pins that decision and INVENTORIES the rest: a newly added
* grouped grid, or a re-spelled filter, changes the inventory and lands here
* for a human read instead of passing silently.
*
* That inventory is also the non-vacuity counter. A walk that stopped seeing
* `grouping` — a renamed key, a refactor of `flattenViews` — would satisfy a
* bare "by_unit is scoped" assertion by finding nothing at all.
*
* ── What the walk reads, and what it deliberately does not ──────────────
* Membership scoping only: `status` + `in` + a value list. A re-spelling that
* excludes the same rows a different way (`status not_in ['done', …]`) reads
* as `(none)` here and fails the pin. That is intended rather than an
* oversight — the two are different decisions with different edges
* (`not_in ['done']` also admits `cancelled` and `skipped`), and a changed
* decision on this lens is exactly the thing that should stop a human.
*
* ── Proven red before the fix ───────────────────────────────────────────
* Run against `src/views/task.view.ts` as it stood at `0f0ec49` — the fix
* committed first, then that one file restored to its pre-fix state, so the
* revert had somewhere to come back from. `npx vitest run
* test/metadata-bindings.test.ts` exited 1 on both assertions; the excerpt is
* in the PR body.
*/
interface GroupedScope {
/** e.g. `view duly_task › listViews.by_unit`. */
readonly where: string;
/** Grouping levels, in the order they nest. */
readonly groupsBy: readonly string[];
/** `status` values the lens is scoped to; empty when it carries no such filter. */
readonly statusScope: readonly string[];
}

/** Every list view that groups, with the status scope it carries. */
export const groupedGridScopes = (views: readonly unknown[]): GroupedScope[] => {
const out: GroupedScope[] = [];

for (const { where, view } of flattenViews(views)) {
const groupsBy = (((view.grouping as Rec | undefined)?.fields as Rec[]) ?? [])
.map((level) => fieldOf(level))
.filter((field): field is string => typeof field === 'string' && field !== '');
if (groupsBy.length === 0) continue;

// `ListViewSchema.filter` is a rule array; anything else is not a scope
// this walk can read, and reads as unscoped rather than being assumed.
const rules = Array.isArray(view.filter) ? (view.filter as Rec[]) : [];
const statusScope = rules
.filter((rule) => rule?.field === 'status' && rule?.operator === 'in')
.flatMap((rule) => (Array.isArray(rule.value) ? (rule.value as unknown[]) : []))
.filter((value): value is string => typeof value === 'string')
.sort();

out.push({ where, groupsBy, statusScope });
}

return out;
};

const scopeLines = (scopes: readonly GroupedScope[]): string[] =>
scopes.map(
(s) => `${s.where} · groups by ${s.groupsBy.join(', ')} · status scope: ${s.statusScope.join(', ') || '(none)'}`,
);

const groupedScopes = groupedGridScopes(stack.views);

describe('the by-unit lens stays scoped to open work (stopgap for objectui#7189)', () => {
it('`by_unit` carries the open-work status filter its grouping depends on', () => {
const byUnit = groupedScopes.find((s) => s.where === 'view duly_task › listViews.by_unit');
expect(byUnit, 'the by-unit lens no longer exists, or no longer groups').toBeDefined();
expect(
byUnit!.statusScope,
'the by-unit lens lost the status filter that keeps its grouping complete. The grid groups over '
+ 'the FETCHED PAGE, so widening this filter puts 151 finished tasks back in front of the open '
+ 'ones, the lens pages again, and a whole business unit drops off the screen with nothing saying '
+ 'a unit is missing — no error, and `validate`, `typecheck`, `test` and `build` all stay green. '
+ 'Restore `status in [\'open\', \'in_progress\']`, and do not reach for a bigger page size: that '
+ 'moves the cliff instead of removing it. See objectstack-ai/objectui#7189.',
).toEqual(['in_progress', 'open']);
});

it('inventories every grouped grid and the scope it carries', () => {
// Doubles as the non-vacuity counter for the assertion above. A NEW
// grouped grid appearing here is not automatically a defect — judge it
// the way `catalog_tree` was judged: does its whole result set fit in one
// page? If it can outgrow one, it needs a scope for its counts to mean
// anything.
expect(
scopeLines(groupedScopes).sort(),
'the set of grouped grids, or the scope one carries, changed — read the note above before updating this',
).toEqual([
'view duly_duty › listViews.catalog_tree · groups by business_unit, owner · status scope: (none)',
'view duly_task › listViews.by_unit · groups by business_unit · status scope: in_progress, open',
]);
});
});

describe('grouped-grid scope guard — the guard can fail (self-test on synthetic metadata)', () => {
const grouped = (overrides: Rec, key = 'lens'): unknown => ({
listViews: {
[key]: {
label: 'Lens',
type: 'grid',
data: { provider: 'object', object: 'fx_task' },
columns: [{ field: 'subject' }, { field: 'business_unit' }],
grouping: { fields: [{ field: 'business_unit' }] },
...overrides,
},
},
});

it('reads the open-work scope off an `in` filter', () => {
const scopes = groupedGridScopes([
grouped({ filter: [{ field: 'status', operator: 'in', value: ['open', 'in_progress'] }] }),
]);
expect(scopes.map((s) => s.statusScope)).toEqual([['in_progress', 'open']]);
});

it('reports `(none)` for a grouped grid carrying no filter at all — the pre-fix by_unit', () => {
const scopes = groupedGridScopes([grouped({})]);
expect(scopeLines(scopes)).toEqual([
'view fx_task › listViews.lens · groups by business_unit · status scope: (none)',
]);
});

it('reports `(none)` when the filter is present but scopes another field', () => {
// `catalog_tree` is unscoped this way too — a filter that narrows on
// something else does not bound the grouped set by status.
const scopes = groupedGridScopes([
grouped({ filter: [{ field: 'due_date', operator: 'less_than', value: '{today}' }] }),
]);
expect(scopes[0]!.statusScope).toEqual([]);
});

it('sees a WIDENED scope as a different scope — this is what catches the regression', () => {
const scopes = groupedGridScopes([
grouped({ filter: [{ field: 'status', operator: 'in', value: ['open', 'in_progress', 'done'] }] }),
]);
expect(scopes[0]!.statusScope).toEqual(['done', 'in_progress', 'open']);
});

it('does not read a `not_in` re-spelling as the same decision', () => {
// Deliberate: `not_in ['done']` also admits `cancelled` and `skipped`, so
// it is a different decision and should stop a human rather than pass.
const scopes = groupedGridScopes([
grouped({ filter: [{ field: 'status', operator: 'not_in', value: ['done'] }] }),
]);
expect(scopes[0]!.statusScope).toEqual([]);
});

it('records every grouping level, so the inventory line names the real shape', () => {
const scopes = groupedGridScopes([
grouped({ grouping: { fields: [{ field: 'business_unit' }, { field: 'owner' }] } }),
]);
expect(scopes[0]!.groupsBy).toEqual(['business_unit', 'owner']);
});

it('examines nothing on a view that does not group', () => {
expect(groupedGridScopes([grouped({ grouping: undefined })])).toEqual([]);
});
});
Loading