mirror of
https://github.com/stablyai/orca.git
synced 2026-10-30 16:47:34 +00:00
Lint: flag store selectors that subscribe to the whole host-status map (#34517)
* lint: flag whole host-status map selectors * lint: keep whole host-status map findings advisory
This commit is contained in:
@@ -22,6 +22,7 @@
|
||||
],
|
||||
"rules": {
|
||||
"app-store-performance/require-selector": "warn",
|
||||
"app-store-performance/no-whole-hot-map-selector": "warn",
|
||||
"app-store-performance/no-identity-selector": "warn",
|
||||
"app-store-performance/no-fresh-selector-result": "warn",
|
||||
"app-store-performance/no-nested-fresh-under-shallow": "warn",
|
||||
|
||||
@@ -321,6 +321,78 @@ function noFreshSelectorResultRule(sourceCode) {
|
||||
})
|
||||
}
|
||||
|
||||
// Host observations replace this map on every meaningful change (runtime-status.ts).
|
||||
const HOT_STORE_FIELDS = new Set(['runtimeStatusByEnvironmentId'])
|
||||
|
||||
function selectorLocalConstant(variable, selector) {
|
||||
const definition = variable?.defs[0]
|
||||
const declaration = definition?.node
|
||||
if (definition?.type !== 'Variable' || declaration.parent.kind !== 'const') {
|
||||
return null
|
||||
}
|
||||
for (let parent = declaration.parent; parent; parent = parent.parent) {
|
||||
if (parent === selector) {
|
||||
return declaration
|
||||
}
|
||||
}
|
||||
return null
|
||||
}
|
||||
|
||||
// Resolve only the selector parameter and its local const aliases, never a same-named outer value.
|
||||
function selectorValue(expression, selector, state, seen = new Set()) {
|
||||
if (expression?.type === 'MemberExpression') {
|
||||
const field = propertyName(expression)
|
||||
return HOT_STORE_FIELDS.has(field) &&
|
||||
selectorValue(expression.object, selector, state, seen) === 'state'
|
||||
? field
|
||||
: null
|
||||
}
|
||||
const variable = variableFor(expression, state)
|
||||
if (!variable || seen.has(variable)) {
|
||||
return null
|
||||
}
|
||||
seen.add(variable)
|
||||
const parameter = selector.params[0]
|
||||
if (parameter?.type === 'Identifier' && variableFor(parameter, state) === variable) {
|
||||
return 'state'
|
||||
}
|
||||
const declaration = selectorLocalConstant(variable, selector)
|
||||
const pattern = declaration?.id ?? parameter
|
||||
if (pattern?.type === 'ObjectPattern') {
|
||||
const property = pattern.properties.find(
|
||||
(entry) => entry.type === 'Property' && variable.identifiers.includes(entry.value)
|
||||
)
|
||||
const field = property && !property.computed ? identifierName(property.key) : null
|
||||
if (
|
||||
HOT_STORE_FIELDS.has(field) &&
|
||||
(pattern === parameter || selectorValue(declaration.init, selector, state, seen) === 'state')
|
||||
) {
|
||||
return field
|
||||
}
|
||||
}
|
||||
return declaration?.id.type === 'Identifier'
|
||||
? selectorValue(declaration.init, selector, state, seen)
|
||||
: null
|
||||
}
|
||||
|
||||
function noWholeHotMapSelectorRule(sourceCode) {
|
||||
return deferredSelectorRule(sourceCode, ({ selector, state }) => {
|
||||
if (!selector) {
|
||||
return null
|
||||
}
|
||||
const result = returnedExpressions(selector).find((expression) =>
|
||||
HOT_STORE_FIELDS.has(selectorValue(expression, selector, state))
|
||||
)
|
||||
return result
|
||||
? {
|
||||
node: result,
|
||||
message:
|
||||
'This whole host-status map changes for every host. Select the derived answer for your own host/item with a stable result (see paired-runtime-parking-capabilities.ts), or add a disable comment with a reason if this consumer needs every host.'
|
||||
}
|
||||
: null
|
||||
})
|
||||
}
|
||||
|
||||
/** useShallow compares one level deep, so a fresh reference nested inside its result never matches. */
|
||||
function nestedFreshValues(expression) {
|
||||
if (expression?.type === 'ObjectExpression') {
|
||||
@@ -389,6 +461,12 @@ function bindContext(createVisitors) {
|
||||
) {
|
||||
return {}
|
||||
}
|
||||
if (
|
||||
createVisitors === noWholeHotMapSelectorRule &&
|
||||
/\.(?:test|spec)\.[^/\\]+$/.test(context.filename)
|
||||
) {
|
||||
return {}
|
||||
}
|
||||
const visitors = createVisitors(context.sourceCode)
|
||||
for (const [nodeType, visit] of Object.entries(visitors)) {
|
||||
visitors[nodeType] = visit.bind(context)
|
||||
@@ -404,6 +482,7 @@ export default {
|
||||
'no-raw-project-catalog-computed': { create: bindContext(noRawProjectCatalogComputedRule) },
|
||||
'no-identity-selector': { create: bindContext(noIdentitySelectorRule) },
|
||||
'no-fresh-selector-result': { create: bindContext(noFreshSelectorResultRule) },
|
||||
'no-whole-hot-map-selector': { create: bindContext(noWholeHotMapSelectorRule) },
|
||||
'no-nested-fresh-under-shallow': { create: bindContext(noNestedFreshUnderShallowRule) }
|
||||
}
|
||||
}
|
||||
|
||||
@@ -2,7 +2,22 @@
|
||||
|
||||
`pnpm --silent audit:perf > performance-audit.json` scans production `src/` with
|
||||
the existing app-store and buffer-concatenation rules. Warnings are advisory in this full inventory; tool/parser failures fail.
|
||||
New warning findings on changed lines fail `pnpm check:code-quality:changed`.
|
||||
New findings from rules enabled in the changed-code configuration fail
|
||||
`pnpm check:code-quality:changed`.
|
||||
|
||||
The `app-store-performance/no-whole-hot-map-selector` warning is advisory only:
|
||||
it appears in this inventory and the performance-contracts artifact, and its
|
||||
findings cannot fail the changed-code gate or other required checks. It catches
|
||||
selectors returning the whole `runtimeStatusByEnvironmentId` map, which is
|
||||
replaced on meaningful host observations. Select a stable derived answer inside
|
||||
the selector (see `paired-runtime-parking-capabilities.ts`). Fleet consumers can
|
||||
optionally silence the inventory warning with
|
||||
`// oxlint-disable-next-line app-store-performance/no-whole-hot-map-selector -- <why every host is needed>`.
|
||||
It resolves direct returns, module-scope named selectors and selector-local const
|
||||
aliases/destructuring; it does not infer consumer lifetime or follow imported
|
||||
helpers. Legitimate fleet consumers also match, so behavior-level render-isolation
|
||||
assertions remain the safeguard against actual fanout regressions.
|
||||
|
||||
Tests, generated files, `mobile/` and `cloud/` are outside this source audit.
|
||||
|
||||
The ast-grep rule `config/ast-grep/rules/no-repeated-collator.yml` (`pnpm lint:ast-grep`,
|
||||
|
||||
@@ -12,6 +12,7 @@ function lintSources(sources) {
|
||||
rules: {
|
||||
'app-store-performance/require-selector': 'warn',
|
||||
'app-store-performance/no-identity-selector': 'warn',
|
||||
'app-store-performance/no-whole-hot-map-selector': 'warn',
|
||||
'app-store-performance/no-fresh-selector-result': 'warn',
|
||||
'app-store-performance/no-nested-fresh-under-shallow': 'warn'
|
||||
}
|
||||
@@ -19,6 +20,117 @@ function lintSources(sources) {
|
||||
}
|
||||
|
||||
const sources = {
|
||||
// The subscription and post-render memo from the typing-lag regression.
|
||||
historicalHotMap: `
|
||||
import { useAppStore } from '@/store'
|
||||
export function useStructuredAgentSessionHostCapabilityState(
|
||||
target: RuntimeClientTarget | null,
|
||||
capability: RuntimeCapability
|
||||
): StructuredAgentSessionHostCapabilityState {
|
||||
const environmentId = target?.kind === 'environment' ? target.environmentId : null
|
||||
const statuses = useAppStore((state) => state.runtimeStatusByEnvironmentId)
|
||||
const remote = useMemo((): StructuredAgentSessionHostCapabilityState => {
|
||||
if (environmentId === null) {
|
||||
return 'unknown'
|
||||
}
|
||||
const entry = statuses.get(environmentId)
|
||||
const status = isRuntimeHostContactRevoked(entry) ? null : lastVerifiedRuntimeStatus(entry)
|
||||
return status
|
||||
? status.capabilities?.includes(capability)
|
||||
? 'supported'
|
||||
: 'unsupported'
|
||||
: 'unknown'
|
||||
}, [statuses, environmentId, capability])
|
||||
const local = useSyncExternalStore(
|
||||
subscribeLocalRuntimeCapabilityObservation,
|
||||
() => localCapabilityState(capability),
|
||||
() => 'unknown' as const
|
||||
)
|
||||
useEffect(() => {
|
||||
if (target?.kind === 'local') {
|
||||
void ensureLocalRuntimeCapabilities()
|
||||
}
|
||||
}, [target?.kind])
|
||||
return !target ? 'unknown' : environmentId === null ? local : remote
|
||||
}
|
||||
`,
|
||||
hotMapReturns: `
|
||||
import { useAppStore as useStore } from '@/store'
|
||||
import { useRuntimeStore } from '@/store/runtime'
|
||||
import { useShallow } from 'zustand/react/shallow'
|
||||
const Direct = () => useStore(s => s.runtimeStatusByEnvironmentId)
|
||||
const Block = () => useStore(state => { return state.runtimeStatusByEnvironmentId })
|
||||
const Alias = () => useStore(state => { const statuses = state.runtimeStatusByEnvironmentId; return statuses })
|
||||
const StateAlias = () => useStore(state => { const snapshot = state; return snapshot.runtimeStatusByEnvironmentId })
|
||||
const Destructure = () => useStore(state => { const { runtimeStatusByEnvironmentId: statuses } = state; return statuses })
|
||||
const Parameter = () => useStore(({ runtimeStatusByEnvironmentId: statuses }) => statuses)
|
||||
const Named = () => useStore(selectStatuses)
|
||||
function selectStatuses(state) { return state.runtimeStatusByEnvironmentId }
|
||||
const Sibling = () => useRuntimeStore(s => s.runtimeStatusByEnvironmentId)
|
||||
const ShallowMap = () => useStore(useShallow(s => s.runtimeStatusByEnvironmentId))
|
||||
`,
|
||||
focusedHotMapReads: `
|
||||
import { useAppStore } from '@/store'
|
||||
import { useShallow } from 'zustand/react/shallow'
|
||||
const Primitive = () => useAppStore(s => s.runtimeStatusByEnvironmentId.get(hostId)?.status !== null)
|
||||
const NonHot = () => useAppStore(s => s.someOtherMap)
|
||||
const Action = () => useAppStore(s => s.action)
|
||||
const onClick = () => useAppStore.getState().runtimeStatusByEnvironmentId
|
||||
const Shallow = () => useAppStore(useShallow(s => ({ online: Boolean(s.runtimeStatusByEnvironmentId.get(hostId)?.status) })))
|
||||
const OtherObject = () => useAppStore(s => other.runtimeStatusByEnvironmentId)
|
||||
const Shadow = () => useAppStore(s => { const statuses = other.runtimeStatusByEnvironmentId; return statuses })
|
||||
function ShadowedHook(useAppStore) { return useAppStore(s => s.runtimeStatusByEnvironmentId) }
|
||||
const ShadowedParameter = () => useAppStore(s => { { const s = other; return s.runtimeStatusByEnvironmentId } })
|
||||
`,
|
||||
fixedHostCapability: `
|
||||
import { useAppStore } from '@/store'
|
||||
export function useStructuredAgentSessionOwnPermissionState(
|
||||
target: RuntimeClientTarget | null
|
||||
): StructuredAgentSessionHostCapabilityState {
|
||||
const capability = AGENT_SESSION_PERMISSION_OWN_RUNTIME_CAPABILITY
|
||||
const environmentId = target?.kind === 'environment' ? target.environmentId : null
|
||||
// Select the verdict, not the map: unrelated host flips must not rerender every chat.
|
||||
const remote = useAppStore((state): StructuredAgentSessionHostCapabilityState => {
|
||||
if (environmentId === null) {
|
||||
return 'unknown'
|
||||
}
|
||||
const entry = state.runtimeStatusByEnvironmentId.get(environmentId)
|
||||
const status = isRuntimeHostContactRevoked(entry) ? null : lastVerifiedRuntimeStatus(entry)
|
||||
return status
|
||||
? status.capabilities?.includes(capability)
|
||||
? 'supported'
|
||||
: 'unsupported'
|
||||
: 'unknown'
|
||||
})
|
||||
const local = useSyncExternalStore(
|
||||
subscribeLocalRuntimeCapabilityObservation,
|
||||
() => localCapabilityState(capability),
|
||||
() => 'unknown' as const
|
||||
)
|
||||
useEffect(() => {
|
||||
if (target?.kind === 'local') {
|
||||
void ensureLocalRuntimeCapabilities()
|
||||
}
|
||||
}, [target?.kind])
|
||||
return !target ? 'unknown' : environmentId === null ? local : remote
|
||||
}
|
||||
|
||||
`,
|
||||
parkingProjection: `
|
||||
import { useAppStore } from '@/store'
|
||||
export function selectPairedRuntimeParkingEnvironmentIdsFromState(state: {
|
||||
runtimeStatusByEnvironmentId: PairedRuntimeParkingCapabilityStatuses
|
||||
}): ReadonlySet<string> {
|
||||
return selectPairedRuntimeParkingEnvironmentIds(state.runtimeStatusByEnvironmentId)
|
||||
}
|
||||
|
||||
const Parking = () => useAppStore(selectPairedRuntimeParkingEnvironmentIdsFromState)
|
||||
`,
|
||||
globalHotMapConsumer: `
|
||||
import { useAppStore } from '@/store'
|
||||
// oxlint-disable-next-line app-store-performance/no-whole-hot-map-selector -- Fleet picker needs every host's current status.
|
||||
const Fleet = () => useAppStore(s => s.runtimeStatusByEnvironmentId)
|
||||
`,
|
||||
escapedImport: String.raw`
|
||||
import { use\u0041ppStore as useStore } from '@/store'
|
||||
export const WholeStore = () => useStore()
|
||||
@@ -121,6 +233,36 @@ beforeAll(() => {
|
||||
})
|
||||
|
||||
describe('app store performance Oxlint plugin', () => {
|
||||
it.each([
|
||||
['historicalHotMap', 1],
|
||||
['hotMapReturns', 9]
|
||||
])('reports whole hot-map results in %s', (name, count) => {
|
||||
expect(diagnosticsBySource.get(sources[name]).map((diagnostic) => diagnostic.code)).toEqual(
|
||||
Array(count).fill('app-store-performance(no-whole-hot-map-selector)')
|
||||
)
|
||||
})
|
||||
|
||||
it.each([
|
||||
'focusedHotMapReads',
|
||||
'fixedHostCapability',
|
||||
'parkingProjection',
|
||||
'globalHotMapConsumer'
|
||||
])('allows stable projections and justified global consumers in %s', (name) => {
|
||||
expect(diagnosticsBySource.get(sources[name])).toEqual([])
|
||||
})
|
||||
|
||||
it('does not flag test files', () => {
|
||||
expect(
|
||||
runOxlintPluginOnSources({
|
||||
pluginName: 'app-store-performance',
|
||||
pluginPath,
|
||||
sources: [sources.historicalHotMap],
|
||||
extension: 'test.tsx',
|
||||
rules: { 'app-store-performance/no-whole-hot-map-selector': 'warn' }
|
||||
})
|
||||
).toEqual([[]])
|
||||
})
|
||||
|
||||
it('resolves an escaped store import through the text prefilter', () => {
|
||||
expect(
|
||||
diagnosticsBySource.get(sources.escapedImport).map((diagnostic) => diagnostic.code)
|
||||
|
||||
Reference in New Issue
Block a user