mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-07 16:35:48 +01:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Tasks and Inbox are the main entry points for that work. > - Earlier changes made their data reads and row counts smaller. > - Those changes remain, but shared controls now pull in the whole task-detail bundle. > - A cold open can also read a remembered organization before selecting the one in the URL. > - This pull request separates collection code and selects the addressed organization first. > - It warms task code after collection paint, so the next task open stays fast. ## Linked Issues or Issue Description Refs #17756, #18323, #16849, #17041, #17983, #17960, #18461. **What happened?** Tasks and Inbox still use their optimized index reads and row budgets. However, small shared controls and hooks were placed in the task-detail chunk. A collection had to download that whole chunk. The HTML also preloaded it on every route. The original production profile loaded about 6.5 MB of decoded JavaScript before useful rows, or 1.9 MB transferred with compression. Company bootstrap could select a different remembered organization before Layout selected the URL's organization. This started unnecessary company reads. A retained multi-company browser fixture reproduced two attention-count requests for different companies. **Expected behavior** Show correctly scoped, stable collection rows without waiting for task-detail code. Keep compact reads, bounded rendering, direct task preloading, immediate New Task input, and fast follow-on navigation. **Steps to reproduce** 1. Use a production UI build with Coordinator enabled and two active organizations. 2. Remember one organization, then cold-open Tasks or Mine in the other. 3. Inspect startup script and company requests before the first useful row. The baseline UI revision is `91fdf02e95`. This is a UI change. It has no API, schema, migration, or authorization changes. ## What Changed - Group the static collection dependency trees above the task-detail group. Keep the existing startup and task grouping strategy. - Preload task detail and its static dependency closure early on direct task and agent-chat URLs. Keep collection aliases out of that preload. - Fetch the immutable task chunk at low priority after a collection paints, or on a same-origin task-link hint. After a successful immutable response is fully read, module-preload the cached chunk so it can compile without executing. A failed HTTP probe skips module preload. - Prefer an addressed active organization during bootstrap. Preserve manual selection, archived-company handling, and account-scoped company lists. - Preserve a task reading anchor when a programmatic scroll event reports an unchanged offset during restore reflow. Strengthen the Back journey to wait for a completed collection navigation. - Add production-browser checks for delayed indexes, correct company scope, stable row geometry, failed speculation, and early direct-task preload. Update collection verification notes and the feature map. ## Verification Final source: `e402ab7fc6`; baseline UI: `91fdf02e95`. - All **234 focused unit cases** and **44 production-browser journeys** passed on the final source: 33 task journeys and 11 collection journeys. Coverage includes compact history, saved cards, draft persistence, avatar continuity, failed-probe recovery, delayed data, pagination, deep links, desktop/mobile and reading-position restoration through Back. Desktop and mobile screenshots were inspected. - Workspace typecheck, full production build, map checks, token gates and diff checks passed. The default local `pnpm test:run` still stops before executing tests because the existing chat-test line selector reports `No test found` (1,816 skipped). This is not a passed full local test suite. Focused tests and remote CI remain separate evidence. - Greptile reviewed the exact final commit at **5/5**, with zero open review threads. Final code CI passed. The complete check inventory is **55 successful, 3 skipped, 1 neutral**. The neutral result is the separate Paperclip Review: its agent finished without completing an assessment, so it is not a passing review score. Five alternating baseline/candidate pairs per scenario, Chromium 151.0.7922.34, 1440×900, 4× CPU, 50ms latency, approximately 9Mbps down / 1.5Mbps up. Cold cases use fresh browser contexts with normal cache writes enabled; warm cases use the same context and normal application query cache. Both previews use the server's immutable hashed-asset cache policy and uncached HTML, with real local feature settings and no request interception. All task-owned builds and suites were stopped during timing; other host activity was uncontrolled. Readiness requires useful rows and editable search, or the readable task thread and editable composer, followed by two animation frames. | Journey | Baseline median (range) | PR median (range) | Median paired saving | | --- | --- | --- | --- | | Cold Tasks | 3239 ms (3189–3333) | 2545 ms (2441–2682) | +752 ms | | Cold Inbox | 3207 ms (3203–3268) | 2679 ms (2502–3062) | +541 ms | | Warm Inbox | 717 ms (701–917) | 760 ms (745–768) | -47 ms | | Warm Tasks | 1150 ms (1126–1207) | 1136 ms (1119–1174) | +18 ms | | First task click | 943 ms (932–1008) | 1007 ms (976–1098) | -74 ms | | Back to Tasks | 584 ms (568–617) | 566 ms (559–575) | +10 ms | | Cold direct task control | 3389 ms (3377–3402) | 3415 ms (3395–3896) | -13 ms | All five pairs improved for each cold collection. The signed saving is baseline minus candidate for each pair; it is distinct from the difference of medians. Warm navigation is broadly similar. First task click was 74ms slower by median paired difference. Direct-task median paired difference was 13ms slower, with one slower 3.90s candidate sample retained in the range; this is not a claimed direct-task speedup. Completed script transfers by collection readiness fell from about **1.91MB to 1.14MB** (Tasks), and **1.90MB to 1.14MB** (Inbox). Task code warms after rows paint. At the test bandwidth, the removed 0.77MB critical transfer represents roughly 0.65s of download time; CPU and network overlap, so their savings are not added. Unthrottled loopback API control medians stayed about 17–18ms in both variants. All 30 cold samples had zero browser errors. Rejected experiments remain separate: automatic splitting produced over 200 startup requests; disabling recursive grouping slowed direct tasks; HTTP-only warming improved collection startup but slowed first task opens. The final correction preloads direct dependencies and compiles successfully cached code after paint. Routing disables HTTP caching, so intercepted runs were kept as diagnostic evidence rather than used for warm-cache acceptance; see [Playwright's cache behavior](https://playwright.dev/docs/api/class-page#page-route). Vite preview's default asset `no-cache` policy was also corrected to match the shipped server before final measurement. ## Risks - Bundle placement affects shared consumers. Browser checks cover collections and task navigation; the full PR checks also cover other routes. - Code warming still transfers the task chunk if the user stays on a populated collection. It moves that download past first paint and does not execute it. This is a startup latency improvement, not a claim that all eventual bytes disappear. - A direct click before warming finishes still loads code on demand. Direct task and chat URLs retain early module preload. - Local browser evidence does not establish a deployed staging speedup. This PR does not merge or deploy itself. ## Model Used OpenAI GPT-6 (Codex), with reasoning, tool use and local code execution. The context-window size is not exposed in this environment. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run the relevant tests locally and they pass (234 unit cases and 44 browser journeys; the full-command limitation is documented above) - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All code CI gates are green (the separate Paperclip Review is neutral because its agent did not complete an assessment) - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
315 lines
12 KiB
TypeScript
315 lines
12 KiB
TypeScript
import {
|
|
createContext,
|
|
useCallback,
|
|
useContext,
|
|
useEffect,
|
|
useMemo,
|
|
useRef,
|
|
useState,
|
|
type Context,
|
|
type ReactNode,
|
|
} from "react";
|
|
import { useMutation, useQueryClient } from "@tanstack/react-query";
|
|
import type { Company } from "@paperclipai/shared";
|
|
import { companiesApi } from "../api/companies";
|
|
import {
|
|
fetchCompanyListForCurrentAccount,
|
|
useAccountIdentity,
|
|
useCompanyListQuery,
|
|
} from "../api/companies-query";
|
|
import { queryKeys } from "../lib/queryKeys";
|
|
import type { CompanySelectionSource } from "../lib/company-selection";
|
|
type CompanySelectionOptions = { source?: CompanySelectionSource };
|
|
|
|
interface CompanyContextValue {
|
|
companies: Company[];
|
|
selectedCompanyId: string | null;
|
|
selectedCompany: Company | null;
|
|
selectionSource: CompanySelectionSource;
|
|
loading: boolean;
|
|
error: Error | null;
|
|
/**
|
|
* There is no usable company list *and* the reason is a failed request rather
|
|
* than an account that owns nothing. Consumers need the two apart: an empty
|
|
* list is a fact to render, this is a dead end to offer a way out of.
|
|
*/
|
|
companyListUnavailable: boolean;
|
|
/** Re-fetches the list for the account signed in now. Pairs with the flag above. */
|
|
retryCompanies: () => Promise<void>;
|
|
setSelectedCompanyId: (companyId: string, options?: CompanySelectionOptions) => void;
|
|
reloadCompanies: () => Promise<void>;
|
|
createCompany: (data: {
|
|
name: string;
|
|
description?: string | null;
|
|
budgetMonthlyCents?: number;
|
|
}) => Promise<Company>;
|
|
}
|
|
|
|
const STORAGE_KEY = "paperclip.selectedCompanyId";
|
|
|
|
// A refresh can replace consumers before a mounted provider is replaced. Keep
|
|
// their context identity in Vite's per-module data; never store account state.
|
|
const CompanyContext: Context<CompanyContextValue | null> =
|
|
import.meta.hot?.data?.companyContext ?? createContext<CompanyContextValue | null>(null);
|
|
if (import.meta.hot?.data) import.meta.hot.data.companyContext = CompanyContext;
|
|
|
|
export function resolveBootstrapCompanySelection(input: {
|
|
companies: Array<Pick<Company, "id">>;
|
|
sidebarCompanies: Array<Pick<Company, "id">>;
|
|
selectedCompanyId: string | null;
|
|
storedCompanyId: string | null;
|
|
routeCompanyId?: string | null;
|
|
}) {
|
|
if (input.companies.length === 0) return null;
|
|
|
|
const selectableCompanies = input.sidebarCompanies.length > 0
|
|
? input.sidebarCompanies
|
|
: input.companies;
|
|
// An already-selected company only needs to EXIST — not to be featured in
|
|
// the sidebar. The Layout route-sync selects whatever company the URL names
|
|
// (archived included, since archived pages are still routable); if this
|
|
// resolver vetoed that selection against the sidebar-filtered list, the two
|
|
// effects would re-select against each other forever and blow React's
|
|
// nested-update limit (the archived-company blank-screen crash). The
|
|
// sidebar filter keeps shaping fresh boots below, where no explicit
|
|
// selection exists yet.
|
|
if (input.selectedCompanyId && input.companies.some((company) => company.id === input.selectedCompanyId)) {
|
|
return input.selectedCompanyId;
|
|
}
|
|
if (input.routeCompanyId && input.sidebarCompanies.some((company) => company.id === input.routeCompanyId)) {
|
|
return input.routeCompanyId;
|
|
}
|
|
if (input.storedCompanyId && selectableCompanies.some((company) => company.id === input.storedCompanyId)) {
|
|
return input.storedCompanyId;
|
|
}
|
|
return selectableCompanies[0]?.id ?? null;
|
|
}
|
|
|
|
export function shouldClearStoredCompanySelection(input: {
|
|
companies: Array<Pick<Company, "id">>;
|
|
isLoading: boolean;
|
|
unauthorized: boolean;
|
|
/**
|
|
* Whether the company request failed. An error is not an answer, and this
|
|
* branch is destructive.
|
|
*
|
|
* `companiesListQueryOptions` sets `retry: false`, and a request that fails
|
|
* before ever succeeding leaves `data` undefined - which the provider
|
|
* defaults to `{ companies: [], unauthorized: false }`. That is
|
|
* indistinguishable from "this account was asked, and owns nothing", so a
|
|
* single failed request on a cold load would clear the customer's stored
|
|
* company and drop them onto whichever company sorts first next time.
|
|
*
|
|
* Not clearing costs nothing: a stored id that no longer resolves is
|
|
* ignored by {@link resolveBootstrapCompanySelection}, which checks it
|
|
* against the current list before using it.
|
|
*/
|
|
errored: boolean;
|
|
}) {
|
|
if (input.errored) return false;
|
|
return !input.isLoading && !input.unauthorized && input.companies.length === 0;
|
|
}
|
|
|
|
export function CompanyProvider({ children }: { children: ReactNode }) {
|
|
const queryClient = useQueryClient();
|
|
const [selectionSource, setSelectionSource] = useState<CompanySelectionSource>("bootstrap");
|
|
const [selectedCompanyId, setSelectedCompanyIdState] = useState<string | null>(null);
|
|
|
|
// Keyed by account, so there is no such thing here as "the list, but whose?".
|
|
// A change of account changes the key, which leaves this observer pending
|
|
// rather than holding the previous account's answer.
|
|
const { data: companiesResult = { companies: [], unauthorized: false }, isLoading, error } =
|
|
useCompanyListQuery({
|
|
// `retry: 1` against the shared `retry: false`, and it is load-bearing
|
|
// here even though an earlier measurement found retries pointless. That
|
|
// measurement was taken against an implementation that removed the cache
|
|
// entry on an account change, which made the observer rebind and issue a
|
|
// second request for free. Keying by account replaced that mechanism, and
|
|
// the free attempt went with it: without this, one blip during a switch
|
|
// leaves the customer with no companies until they find Try again.
|
|
retry: 1,
|
|
});
|
|
const companies = companiesResult.companies;
|
|
const companyListUnauthorized = companiesResult.unauthorized;
|
|
const sidebarCompanies = useMemo(
|
|
() => companies.filter((company) => company.status !== "archived"),
|
|
[companies],
|
|
);
|
|
|
|
// The list is account-keyed, but the *selection* is component state and does
|
|
// not change key with it. It belongs to the account that just went away, so it
|
|
// is dropped here. The stored id deliberately survives:
|
|
// resolveBootstrapCompanySelection re-validates it against the incoming list,
|
|
// so an account signing back in keeps its company while an unrelated account
|
|
// cannot inherit it.
|
|
const { userId: sessionUserId, settled: isSessionSettled } = useAccountIdentity();
|
|
const observedUserIdRef = useRef<string | null | undefined>(undefined);
|
|
|
|
useEffect(() => {
|
|
// Until the session settles the account is unknown, not changed.
|
|
if (!isSessionSettled) return;
|
|
const previousUserId = observedUserIdRef.current;
|
|
observedUserIdRef.current = sessionUserId;
|
|
// First settled observation is this tab's boot: no previous account to leave.
|
|
if (previousUserId === undefined || previousUserId === sessionUserId) return;
|
|
|
|
setSelectedCompanyIdState(null);
|
|
setSelectionSource("bootstrap");
|
|
}, [isSessionSettled, sessionUserId]);
|
|
|
|
// Auto-select first company when list loads
|
|
useEffect(() => {
|
|
// `isLoading` covers the account change too: the key moved, so this observer
|
|
// is pending on a list for the new account rather than holding the old one.
|
|
if (isLoading) return;
|
|
// An errored list says nothing about which companies this account has, and
|
|
// `retry: false` makes a single network blip stick. Treat it as undecided
|
|
// rather than as "no companies", which would clear the stored selection.
|
|
if (error) return;
|
|
if (companies.length === 0) {
|
|
if (shouldClearStoredCompanySelection({
|
|
companies,
|
|
isLoading: false,
|
|
unauthorized: companyListUnauthorized,
|
|
errored: error !== null,
|
|
})) {
|
|
if (selectedCompanyId !== null) {
|
|
setSelectedCompanyIdState(null);
|
|
}
|
|
localStorage.removeItem(STORAGE_KEY);
|
|
}
|
|
return;
|
|
}
|
|
|
|
const next = resolveBootstrapCompanySelection({
|
|
companies,
|
|
sidebarCompanies,
|
|
selectedCompanyId,
|
|
storedCompanyId: localStorage.getItem(STORAGE_KEY),
|
|
routeCompanyId: companies.find(company =>
|
|
company.issuePrefix.toLowerCase() === window.location.pathname.split("/")[1]?.toLowerCase(),
|
|
)?.id,
|
|
});
|
|
if (next === null || next === selectedCompanyId) return;
|
|
setSelectedCompanyIdState(next);
|
|
setSelectionSource("bootstrap");
|
|
localStorage.setItem(STORAGE_KEY, next);
|
|
}, [
|
|
companies,
|
|
companyListUnauthorized,
|
|
error,
|
|
isLoading,
|
|
selectedCompanyId,
|
|
sidebarCompanies,
|
|
]);
|
|
|
|
const setSelectedCompanyId = useCallback((companyId: string, options?: CompanySelectionOptions) => {
|
|
setSelectedCompanyIdState(companyId);
|
|
setSelectionSource(options?.source ?? "manual");
|
|
localStorage.setItem(STORAGE_KEY, companyId);
|
|
}, []);
|
|
|
|
const reloadCompanies = useCallback(async () => {
|
|
await queryClient.invalidateQueries({ queryKey: queryKeys.companies.all });
|
|
}, [queryClient]);
|
|
|
|
// The way out of the dead end. Not `reloadCompanies`: invalidation refetches
|
|
// only through a mounted observer, and it leaves an errored query reporting
|
|
// its old error, so the recovery affordance would keep telling the customer
|
|
// the list is unavailable after a retry had already succeeded.
|
|
const retryCompanies = useCallback(async () => {
|
|
try {
|
|
await fetchCompanyListForCurrentAccount(queryClient);
|
|
} catch {
|
|
// Not swallowed — `fetchQuery` records it on the query, which is where
|
|
// `companyListUnavailable` below reads it from. Caught only so the
|
|
// rejection does not go unhandled.
|
|
}
|
|
}, [queryClient]);
|
|
|
|
// Empty because we could not find out, as opposed to empty because the account
|
|
// owns nothing. Derived from the query rather than tracked alongside it: the
|
|
// query is the only thing that knows whether the last attempt succeeded, and a
|
|
// second copy of that answer drifts — it did, reporting a failure over a later
|
|
// empty list that was simply the truth.
|
|
const companyListUnavailable = companies.length === 0 && Boolean(error);
|
|
|
|
const createMutation = useMutation({
|
|
mutationFn: (data: {
|
|
name: string;
|
|
description?: string | null;
|
|
budgetMonthlyCents?: number;
|
|
}) =>
|
|
companiesApi.create(data),
|
|
onSuccess: (company) => {
|
|
queryClient.invalidateQueries({ queryKey: queryKeys.companies.all });
|
|
setSelectedCompanyId(company.id);
|
|
},
|
|
});
|
|
|
|
const createCompany = useCallback(
|
|
async (data: {
|
|
name: string;
|
|
description?: string | null;
|
|
budgetMonthlyCents?: number;
|
|
}) => {
|
|
return createMutation.mutateAsync(data);
|
|
},
|
|
[createMutation],
|
|
);
|
|
|
|
const selectedCompany = useMemo(
|
|
() => companies.find((company) => company.id === selectedCompanyId) ?? null,
|
|
[companies, selectedCompanyId],
|
|
);
|
|
|
|
const value = useMemo(
|
|
() => ({
|
|
companies,
|
|
selectedCompanyId,
|
|
selectedCompany,
|
|
selectionSource,
|
|
loading: isLoading,
|
|
error: error as Error | null,
|
|
companyListUnavailable,
|
|
retryCompanies,
|
|
setSelectedCompanyId,
|
|
reloadCompanies,
|
|
createCompany,
|
|
}),
|
|
[
|
|
companies,
|
|
selectedCompanyId,
|
|
selectedCompany,
|
|
selectionSource,
|
|
isLoading,
|
|
error,
|
|
companyListUnavailable,
|
|
retryCompanies,
|
|
setSelectedCompanyId,
|
|
reloadCompanies,
|
|
createCompany,
|
|
],
|
|
);
|
|
|
|
return <CompanyContext.Provider value={value}>{children}</CompanyContext.Provider>;
|
|
}
|
|
|
|
export function useCompany() {
|
|
const ctx = useContext(CompanyContext);
|
|
if (!ctx) {
|
|
throw new Error("useCompany must be used within CompanyProvider");
|
|
}
|
|
return ctx;
|
|
}
|
|
|
|
/**
|
|
* Non-throwing variant of {@link useCompany}. Returns null when called outside a
|
|
* CompanyProvider instead of throwing, so components that may render in
|
|
* provider-less surfaces (e.g. exported/standalone markdown) can read company
|
|
* state without crashing.
|
|
*/
|
|
export function useOptionalCompany(): CompanyContextValue | null {
|
|
return useContext(CompanyContext);
|
|
}
|