|
| 1 | +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. |
| 2 | +// |
| 3 | +// [#8786] `searchAll`'s TITLE rendering reads the canonical primary-title |
| 4 | +// pointer, not only the deprecated alias. |
| 5 | +// |
| 6 | +// ## The defect |
| 7 | +// |
| 8 | +// `renderTitle`'s candidate list opened with `obj.displayNameField` alone. |
| 9 | +// Under ADR-0079 `nameField` is the CANONICAL pointer and `displayNameField` |
| 10 | +// is the deprecated alias — and `provisionPrimary`, the designation seat the |
| 11 | +// SchemaRegistry runs on every object at registration, stamps `nameField` |
| 12 | +// ONLY (`packages/spec/src/data/display-name.ts:301,314`). So an object whose |
| 13 | +// primary title is designated canonically, without the deprecated alias |
| 14 | +// alongside it, produced `undefined` for that entry, the entry was filtered |
| 15 | +// out, and the title fell through the conventional-name list to |
| 16 | +// `String(row.id)` — the ⌘K palette showing a raw record id where the |
| 17 | +// object's own declared, populated title existed. |
| 18 | +// |
| 19 | +// ## Why the fixtures look the way they do |
| 20 | +// |
| 21 | +// The impact is bounded to objects whose primary title is OUTSIDE |
| 22 | +// `name` / `full_name` / `title` / `subject` / `label` / `company`: anything |
| 23 | +// in that list resolves through the conventional entries and titles |
| 24 | +// identically before and after the fix. Every pin here that must discriminate |
| 25 | +// therefore uses a NON-conventional title field (`company_name`, `ref_no`, |
| 26 | +// `ref_code` — note `company_name` is not `company`). |
| 27 | +// |
| 28 | +// ## Recall is NOT what this file pins |
| 29 | +// |
| 30 | +// #7643 fixed which rows come back and delegated recall to the engine; that |
| 31 | +// contract is pinned in `protocol.search-case-fold.test.ts`. This file is |
| 32 | +// PRESENTATION — what the returned row is called. Same function, different |
| 33 | +// contract. The double below is deliberately a boundary stand-in with no |
| 34 | +// filtering: it returns its rows verbatim so the assertions are about |
| 35 | +// titling and nothing else. |
| 36 | + |
| 37 | +import { describe, it, expect, vi } from 'vitest'; |
| 38 | +import { ObjectStackProtocolImplementation } from './protocol.js'; |
| 39 | + |
| 40 | +type ObjectMeta = Record<string, unknown> & { name: string }; |
| 41 | + |
| 42 | +/** |
| 43 | + * A protocol over a fixed object set. `rowsByObject` is served verbatim — |
| 44 | + * no filtering, no `$search` expansion — so a hit's `title` is the only |
| 45 | + * thing under test. |
| 46 | + */ |
| 47 | +function makeProtocol(objects: ObjectMeta[], rowsByObject: Record<string, unknown[]>) { |
| 48 | + const find = vi.fn(async (object: string) => rowsByObject[object] ?? []); |
| 49 | + const engine = { |
| 50 | + registry: { |
| 51 | + getObject: (n: string) => objects.find(o => o.name === n), |
| 52 | + getAllObjects: () => objects, |
| 53 | + }, |
| 54 | + find, |
| 55 | + }; |
| 56 | + return new ObjectStackProtocolImplementation(engine as never); |
| 57 | +} |
| 58 | + |
| 59 | +const text = (name: string) => ({ name, type: 'text' }); |
| 60 | + |
| 61 | +describe('[#8786] searchAll titles a hit from the canonical nameField', () => { |
| 62 | + it('titles from a canonically-designated nameField with no displayNameField', async () => { |
| 63 | + // THE PIN. `account` declares its primary title the way |
| 64 | + // `provisionPrimary` designates it — `nameField` only, no deprecated |
| 65 | + // alias — and `company_name` is outside the conventional list, so |
| 66 | + // before the fix this hit came back titled `acc_1`. |
| 67 | + const account: ObjectMeta = { |
| 68 | + name: 'account', |
| 69 | + nameField: 'company_name', |
| 70 | + fields: { company_name: text('company_name') }, |
| 71 | + }; |
| 72 | + const p = makeProtocol([account], { |
| 73 | + account: [{ id: 'acc_1', company_name: 'Acme Industrial' }], |
| 74 | + }); |
| 75 | + |
| 76 | + const { hits } = await p.searchAll({ q: 'acme', perObject: 5 }); |
| 77 | + |
| 78 | + expect(hits).toHaveLength(1); |
| 79 | + expect(hits[0].title).toBe('Acme Industrial'); |
| 80 | + // Stated separately: the regression's signature is the raw id, and an |
| 81 | + // assertion naming it survives a future refactor of the value above. |
| 82 | + expect(hits[0].title).not.toBe('acc_1'); |
| 83 | + }); |
| 84 | + |
| 85 | + it('CONTROL — an object using a conventional `name` still titles correctly', async () => { |
| 86 | + // The discriminating control. Without it the pin above cannot tell |
| 87 | + // "the candidate list now reads the canonical pointer" from "the |
| 88 | + // candidate list was bypassed": a short-circuit on the pointer would |
| 89 | + // pass the pin and break this, since `contact` declares no pointer at |
| 90 | + // all and depends entirely on the conventional entries. |
| 91 | + const contact: ObjectMeta = { |
| 92 | + name: 'contact', |
| 93 | + fields: { name: text('name') }, |
| 94 | + }; |
| 95 | + const p = makeProtocol([contact], { |
| 96 | + contact: [{ id: 'con_1', name: 'Ada Lovelace' }], |
| 97 | + }); |
| 98 | + |
| 99 | + const { hits } = await p.searchAll({ q: 'ada', perObject: 5 }); |
| 100 | + |
| 101 | + expect(hits).toHaveLength(1); |
| 102 | + expect(hits[0].title).toBe('Ada Lovelace'); |
| 103 | + }); |
| 104 | + |
| 105 | + it('CONTROL — the pointer is a CANDIDATE, not a short-circuit: an empty value falls through', async () => { |
| 106 | + // The second half of the same discrimination. `nameField` names a |
| 107 | + // field that exists but is blank on this row; the ordered list must |
| 108 | + // continue to the conventional `name` rather than returning empty or |
| 109 | + // dropping to the id. A `return row[pointer]` rewrite passes both |
| 110 | + // tests above and fails here. |
| 111 | + const account: ObjectMeta = { |
| 112 | + name: 'account', |
| 113 | + nameField: 'company_name', |
| 114 | + fields: { company_name: text('company_name'), name: text('name') }, |
| 115 | + }; |
| 116 | + const p = makeProtocol([account], { |
| 117 | + account: [{ id: 'acc_2', company_name: ' ', name: 'Fallback Co' }], |
| 118 | + }); |
| 119 | + |
| 120 | + const { hits } = await p.searchAll({ q: 'fallback', perObject: 5 }); |
| 121 | + |
| 122 | + expect(hits).toHaveLength(1); |
| 123 | + expect(hits[0].title).toBe('Fallback Co'); |
| 124 | + }); |
| 125 | + |
| 126 | + it('still honors the deprecated displayNameField alias on its own', async () => { |
| 127 | + // Back-compat: the alias is DEPRECATED, not withdrawn. An object |
| 128 | + // carrying only the alias titled correctly before this change and |
| 129 | + // must keep doing so — the fix adds a canonical read in front of the |
| 130 | + // alias, it does not replace it. |
| 131 | + const legacy: ObjectMeta = { |
| 132 | + name: 'legacy_doc', |
| 133 | + displayNameField: 'ref_code', |
| 134 | + fields: { ref_code: text('ref_code') }, |
| 135 | + }; |
| 136 | + const p = makeProtocol([legacy], { |
| 137 | + legacy_doc: [{ id: 'leg_1', ref_code: 'DOC-2291' }], |
| 138 | + }); |
| 139 | + |
| 140 | + const { hits } = await p.searchAll({ q: 'doc', perObject: 5 }); |
| 141 | + |
| 142 | + expect(hits).toHaveLength(1); |
| 143 | + expect(hits[0].title).toBe('DOC-2291'); |
| 144 | + }); |
| 145 | + |
| 146 | + it('prefers nameField when an object carries BOTH pointers naming different fields', async () => { |
| 147 | + // The precedence this change lands on, pinned rather than assumed. |
| 148 | + // It is the platform's existing answer, not a new one: |
| 149 | + // `resolveDisplayField` spells `nameField ?? displayNameField` |
| 150 | + // (`spec/src/data/display-name.ts`) and is pinned to prefer |
| 151 | + // `nameField` in `display-name.test.ts`; `searchAll`'s own |
| 152 | + // search-field resolution and the #4254 ingress gate spell it the same |
| 153 | + // way. A fifth answer here would re-split what those merged. |
| 154 | + // |
| 155 | + // No object in the repo is actually in this state — every real object |
| 156 | + // carrying both spells them identically (measured across the repo for |
| 157 | + // #8786) — so this pins the rule for the first object that isn't, |
| 158 | + // rather than changing any object's behaviour today. |
| 159 | + const ticket: ObjectMeta = { |
| 160 | + name: 'ticket', |
| 161 | + nameField: 'ref_no', |
| 162 | + displayNameField: 'legacy_ref', |
| 163 | + fields: { ref_no: text('ref_no'), legacy_ref: text('legacy_ref') }, |
| 164 | + }; |
| 165 | + const p = makeProtocol([ticket], { |
| 166 | + ticket: [{ id: 'tk_1', ref_no: 'TK-77', legacy_ref: 'OLD-11' }], |
| 167 | + }); |
| 168 | + |
| 169 | + const { hits } = await p.searchAll({ q: 'tk', perObject: 5 }); |
| 170 | + |
| 171 | + expect(hits).toHaveLength(1); |
| 172 | + expect(hits[0].title).toBe('TK-77'); |
| 173 | + }); |
| 174 | + |
| 175 | + it('titleFormat still outranks the pointer', async () => { |
| 176 | + // Ordering above the changed line is untouched: `titleFormat` is |
| 177 | + // resolved before the candidate list is built, so a canonical |
| 178 | + // `nameField` does not overtake an explicit format. |
| 179 | + const account: ObjectMeta = { |
| 180 | + name: 'account', |
| 181 | + titleFormat: '{company_name} ({region})', |
| 182 | + nameField: 'company_name', |
| 183 | + fields: { company_name: text('company_name'), region: text('region') }, |
| 184 | + }; |
| 185 | + const p = makeProtocol([account], { |
| 186 | + account: [{ id: 'acc_3', company_name: 'Acme Industrial', region: 'EMEA' }], |
| 187 | + }); |
| 188 | + |
| 189 | + const { hits } = await p.searchAll({ q: 'acme', perObject: 5 }); |
| 190 | + |
| 191 | + expect(hits).toHaveLength(1); |
| 192 | + expect(hits[0].title).toBe('Acme Industrial (EMEA)'); |
| 193 | + }); |
| 194 | +}); |
0 commit comments