feat(tasks): add inline assignees and due dates to page tasks - #4
Conversation
…ysis - add exploration 0102 covering AFFiNE BlockSuite integration feasibility - document architecture fit, risks, and implementation considerations - preserve findings in docs/explorations for team review
- explore page-native tasks backed by first-class Task nodes - compare Notion, Linear, AFFiNE, and Obsidian task models - recommend smart references for GitHub, Figma, YouTube, and similar links - outline phased implementation, validation, and example schema direction
- add inline smart task references in the editor for supported URLs - introduce ExternalReferenceSchema for normalized external artifact metadata - expand TaskSchema with page binding, assignees, source, sort key, and references - cover editor smart-reference behavior and new data schemas with focused tests
- add a page task item extension that persists task ids and block ids in TipTap rows - reconcile page task snapshots into Task and ExternalReference nodes for web and desktop page views - add focused tests and mark completed items in the original task exploration checklist
- add rich-text @mention suggestions that store canonical DIDs in inline task chips - add inline due-date chips and sync both assignees and due dates into page-backed Task nodes - extend task metadata tests and check off the completed exploration checklist items
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds page-backed task primitives, editor task UI (mentions, due dates, task views, smart references), a debounced usePageTaskSync hook reconciling editor snapshots with canonical Task/ExternalReference records, schema/types/tests, gravatar utilities, UI panels (web/electron), and CI workflows for PR previews and site deploys. Changes
Sequence DiagramsequenceDiagram
participant User
participant Editor as RichTextEditor
participant Ext as PageTaskExtension
participant Hook as usePageTaskSync
participant API as Task/ExternalReference API
User->>Editor: edit task content (title, `@mention`, due date, embed)
Editor->>Ext: collectPageTasks() -> snapshot + attribute updates
Ext-->>Editor: snapshot (and optional tx updates)
Editor->>Hook: onPageTasksChange(snapshot)
Note over Hook: debounced aggregation
Hook->>API: create/update/restore/remove Tasks & upsert ExternalReferences
API-->>Hook: responses
Hook-->>Editor: sync status / error
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Greptile SummaryThis PR introduces inline Key points:
Confidence Score: 3/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant E as RichTextEditor
participant PT as PageTaskItemExtension
participant RTM as TaskMentionExtension
participant RTD as TaskDueDateExtension
participant SR as SmartReferenceExtension
participant PTS as usePageTaskSync
participant Q as useQuery / useMutate
participant DB as NodeStorage
E->>PT: editor 'update' event
PT->>E: collectPageTasks(doc) → PageTaskSnapshot[]
E->>PTS: onPageTasksChange(snapshots)
Note over PTS: debounce 250 ms
PTS->>Q: useQuery(TaskSchema, { where: { page } })
Q-->>PTS: existingTasks[]
PTS->>PTS: diff snapshots vs existingTasks
PTS->>DB: upsert ExternalReference nodes (refs first)
PTS->>DB: restore soft-deleted tasks
PTS->>DB: create new Task nodes
PTS->>DB: update changed Task nodes
PTS->>DB: soft-delete removed tasks
DB-->>Q: reactive update
Q-->>E: TaskCollectionEmbed / PageTasksPanel re-renders
Note over E,RTM: @mention flow
E->>RTM: user types '@'
RTM->>E: getSuggestions() from mentionSuggestionsRef
RTM-->>E: TaskMentionMenu dropdown
Note over E,SR: paste flow
E->>SR: handlePaste (inside taskItem)
SR->>SR: parseSmartReferenceUrl(text)
SR-->>E: insert smartReference chip
Last reviewed commit: 9f55dd5 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| function arraysEqual(a: string[] | undefined, b: string[]): boolean { | ||
| if (!Array.isArray(a)) return b.length === 0 | ||
| if (a.length !== b.length) return false | ||
|
|
||
| for (let index = 0; index < a.length; index += 1) { | ||
| if (a[index] !== b[index]) return false | ||
| } | ||
|
|
||
| return true | ||
| } |
There was a problem hiding this comment.
arraysEqual is order-sensitive, triggering spurious updates when assignee order differs
normalizeAssignees uses Set (insertion-order, not sorted), so ['did:a', 'did:b'] and ['did:b', 'did:a'] produce two different arrays. If the editor snapshot delivers assignees in a different order than was previously stored, arraysEqual returns false and an update is written even though the set of assignees hasn't changed. Over time — especially with multiple concurrent editors — this could produce high-frequency unnecessary writes.
Sorting before comparison (or sorting inside normalizeAssignees) would make this stable:
| function arraysEqual(a: string[] | undefined, b: string[]): boolean { | |
| if (!Array.isArray(a)) return b.length === 0 | |
| if (a.length !== b.length) return false | |
| for (let index = 0; index < a.length; index += 1) { | |
| if (a[index] !== b[index]) return false | |
| } | |
| return true | |
| } | |
| function arraysEqual(a: string[] | undefined, b: string[]): boolean { | |
| if (!Array.isArray(a)) return b.length === 0 | |
| if (a.length !== b.length) return false | |
| const sortedA = [...a].sort() | |
| const sortedB = [...b].sort() | |
| for (let index = 0; index < sortedA.length; index += 1) { | |
| if (sortedA[index] !== sortedB[index]) return false | |
| } | |
| return true | |
| } |
- switch production site publishing from artifact-based Pages deploys to a gh-pages branch - add pull request preview publishing under pr/<number>/app with PR comments - preserve production content while updating previews and clean preview folders on PR close - align the manual undeploy workflow with the new branch-backed Pages model
- publish the current production site tree to gh-pages - publish PR #4 web preview under pr/4/app - add the static files needed for custom-domain Pages hosting
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (12)
packages/editor/src/components/TaskMentionMenu.tsx (2)
4-18: Consider usingtypeinstead ofinterfacefor object shapes.As per coding guidelines, prefer
typeoverinterfacefor defining object shapes in TypeScript.♻️ Suggested refactor
-export interface TaskMentionSuggestion { +export type TaskMentionSuggestion = { id: string label: string subtitle?: string color?: string } -interface TaskMentionMenuProps { +type TaskMentionMenuProps = { items: TaskMentionSuggestion[] command: (item: TaskMentionSuggestion) => void } -export interface TaskMentionMenuRef { +export type TaskMentionMenuRef = { onKeyDown: (event: KeyboardEvent) => boolean }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/editor/src/components/TaskMentionMenu.tsx` around lines 4 - 18, Replace the exported and local interfaces with type aliases: change TaskMentionSuggestion, TaskMentionMenuProps, and TaskMentionMenuRef from interface declarations to equivalent type declarations so the object shapes use type instead of interface; ensure the exported name TaskMentionSuggestion remains exported and that the signatures for command and onKeyDown keep the same types.
1-3: Consider adding file-level JSDoc.As per coding guidelines for
packages/**/*.{ts,tsx}, include a file-level JSDoc at the top of the file.📝 Suggested fix
+/** + * `@xnetjs/editor` - Task mention menu component + * + * Provides a keyboard-navigable dropdown for selecting assignees in task mentions. + */ import { forwardRef, useCallback, useEffect, useImperativeHandle, useState } from 'react'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/editor/src/components/TaskMentionMenu.tsx` around lines 1 - 3, Add a file-level JSDoc comment at the top of TaskMentionMenu.tsx describing the module's purpose and exported component(s) (e.g., that this file implements the TaskMentionMenu React component used for inserting task mentions), including any relevant tags like `@module` and a brief summary; place the JSDoc immediately above the existing imports so it applies to the whole file and keep it concise per the repository guidelines.docs/explorations/0103_[_]_TASKS_EMBEDDED_IN_PAGES_BACKED_BY_NODES_MENTIONS_DUE_DATES_NESTED_SUBTASKS_DATABASES_CANVASES_AND_CROSS_SURFACE_TASK_MODEL.md (1)
220-220: Minor: Fix space in code span.The markdown linter flagged spaces inside the code span. Consider removing the spaces around
[].📝 Suggested fix
-- type `[] `, +- type `[]`,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/explorations/0103_`[_]_TASKS_EMBEDDED_IN_PAGES_BACKED_BY_NODES_MENTIONS_DUE_DATES_NESTED_SUBTASKS_DATABASES_CANVASES_AND_CROSS_SURFACE_TASK_MODEL.md at line 220, The markdown has an extra space inside the code span around the square brackets — update the span containing `[] ` so it has no internal trailing/leading spaces (change `[] ` to `[]`) where the fragment reads "type `[] `," to satisfy the linter; search for the code span `[] ` and remove the space so it becomes `[]`.apps/electron/src/renderer/components/PageView.tsx (1)
105-129: Consider usingfor...ofto avoid lint warning on forEach callback.The
addSuggestionfunction has an early return (line 115) which causes the Biome lint warning about returning a value from aforEachcallback. While the logic is correct, usingfor...ofis cleaner and avoids the warning.♻️ Suggested refactor
- addSuggestion(did ? { did } : null, true) - presence.forEach((user) => addSuggestion(user)) + addSuggestion(did ? { did } : null, true) + for (const user of presence) { + addSuggestion(user) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/electron/src/renderer/components/PageView.tsx` around lines 105 - 129, The Biome lint warning comes from using presence.forEach with addSuggestion which has an early return; change the iteration to a for...of loop to avoid returning from a forEach callback. In the memo block around mentionSuggestions, replace the presence.forEach(...) call with a simple for (const user of presence) call that invokes addSuggestion(user) (keep addSuggestion, presence, did and mentionSuggestions names intact) so behavior is unchanged but the lint warning is eliminated.packages/editor/src/extensions/smart-reference/SmartReferenceExtension.test.ts (1)
1-1: Add the required file-level JSDoc header.Line 1 starts with imports; this new package test file should include a top-level JSDoc banner per repo standard.
As per coding guidelines,
packages/**/*.{ts,tsx}: Include file-level JSDoc at top of file:/**@xnetjs/sync- Unified sync primitives for xNet */.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/editor/src/extensions/smart-reference/SmartReferenceExtension.test.ts` at line 1, Add the required file-level JSDoc banner at the very top of the test file before any imports; insert the exact header /** `@xnetjs/sync` - Unified sync primitives for xNet */ above the existing import statement (the file currently imports Editor from '@tiptap/core') so the file complies with the packages/**/*.{ts,tsx} guideline and the package-level standard.packages/react/src/hooks/usePageTaskSync.test.tsx (1)
1-1: Add the required file-level JSDoc header.Line 1 starts immediately with imports; this new
packages/*file should include the repository’s file-level JSDoc banner.As per coding guidelines,
packages/**/*.{ts,tsx}: Include file-level JSDoc at top of file:/**@xnetjs/sync- Unified sync primitives for xNet */.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react/src/hooks/usePageTaskSync.test.tsx` at line 1, Add the repository’s required file-level JSDoc banner at the very top of the file before the first import (currently "import type { DID } from '@xnetjs/core'"); insert the exact header /** `@xnetjs/sync` - Unified sync primitives for xNet */ as the first line so the file complies with the packages/**/*.{ts,tsx} guideline.packages/editor/src/extensions/task-metadata/TaskDueDateExtension.ts (1)
1-4: Add package header JSDoc and prefertypefor options shape.As per coding guidelines, "Include file-level JSDoc at top of file:
/**@xnetjs/sync- Unified sync primitives for xNet */" and "Prefertypeoverinterfacefor defining object shapes in TypeScript".Also applies to: 102-104
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/editor/src/extensions/task-metadata/TaskDueDateExtension.ts` around lines 1 - 4, Add the required file-level JSDoc header at the top of TaskDueDateExtension.ts (e.g. /** `@xnetjs/sync` - Unified sync primitives for xNet */) and replace any exported interface used for options (e.g. TaskDueDateOptions or similar options interface/type declarations) with a TypeScript `type` alias instead of `interface`; update any references to that symbol (constructor signatures, defaultOptions, getOptions, or extension declaration) to use the new type name so the file follows the coding guideline preferring `type` for object shapes.packages/editor/src/extensions/smart-reference/SmartReferenceExtension.ts (1)
11-47: Align declarations with naming/type guidelines (SCREAMING_SNAKE_CASE+type).Rename
SmartReferencePastePluginKeyto SCREAMING_SNAKE_CASE and convertSmartReferenceOptionsto atypealias.As per coding guidelines, "Constants must use SCREAMING_SNAKE_CASE:
MAX_UPDATE_SIZE,DEFAULT_TTL" and "Prefertypeoverinterfacefor defining object shapes in TypeScript".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/editor/src/extensions/smart-reference/SmartReferenceExtension.ts` around lines 11 - 47, Rename the PluginKey constant SmartReferencePastePluginKey to SCREAMING_SNAKE_CASE (e.g., SMART_REFERENCE_PASTE_PLUGIN_KEY) and update every reference to that identifier (including where new PluginKey('smartReferencePaste') is created and any uses/imports/exports). Also convert the exported interface SmartReferenceOptions into a type alias (e.g., type SmartReferenceOptions = { autoConvertTaskPaste: boolean; HTMLAttributes: Record<string, string>; }) and update any places that reference the interface to use the new type alias. Ensure exports and any imports elsewhere are updated to the new names to keep types and runtime identifiers consistent.packages/editor/src/extensions/smart-reference/providers.ts (1)
21-31: Use atypealias forSmartReferenceobject shape.As per coding guidelines, "Prefer
typeoverinterfacefor defining object shapes in TypeScript".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/editor/src/extensions/smart-reference/providers.ts` around lines 21 - 31, Replace the exported interface SmartReference with a type alias that preserves the exact object shape; change the declaration from "export interface SmartReference { ... }" to "export type SmartReference = { ... }" keeping all fields (provider, kind: SmartReferenceKind, url, refId, title, subtitle?, icon, embedUrl?, metadata: Record<string,string>) and export, and then run a quick grep for any places that rely on interface-specific features (like declaration merging or "implements") and adjust those usages if necessary.packages/editor/src/extensions/page-tasks/index.ts (1)
1-27: Add package JSDoc header and switch snapshot shapes totypealiases.As per coding guidelines, "Include file-level JSDoc at top of file:
/**@xnetjs/sync- Unified sync primitives for xNet */" and "Prefertypeoverinterfacefor defining object shapes in TypeScript".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/editor/src/extensions/page-tasks/index.ts` around lines 1 - 27, Add the required file-level JSDoc header exactly as /** `@xnetjs/sync` - Unified sync primitives for xNet */ at the very top of the file, and convert the exported interfaces PageTaskReferenceSnapshot and PageTaskSnapshot into type aliases with identical object shapes (replace "export interface PageTaskReferenceSnapshot { ... }" and "export interface PageTaskSnapshot { ... }" with "export type PageTaskReferenceSnapshot = { ... }" and "export type PageTaskSnapshot = { ... }" respectively), keeping all property names and types unchanged.packages/editor/src/components/RichTextEditor.tsx (1)
4-44: Reorder imports to keep external imports before internal/local imports.As per coding guidelines, "Internal package imports should follow external packages:
import { hash } from '@xnetjs/crypto'" and "Local relative imports should come last:import { helper } from './utils'".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/editor/src/components/RichTextEditor.tsx` around lines 4 - 44, Move all external/package imports to the top and place the local/internal imports (everything imported from '../extensions') after them: keep imports from '@tiptap/*', 'y-protocols/awareness', 'yjs', 'react', etc. at the top, then import the types and symbols coming from '../extensions' (DatabaseViewType, PageTaskSnapshot, SlashCommandItem, TaskMentionSuggestion, Wikilink, LivePreview, HeadingWithSyntax, CodeBlockWithSyntax, BlockquoteWithSyntax, SlashCommand, DragHandleExtension, KeyboardShortcutsExtension, ImageExtension, CalloutExtension, ToggleExtension, FileExtension, SmartReferenceExtension, EmbedExtension, DatabaseEmbedExtension, PageTaskItemExtension, TaskMentionExtension, TaskDueDateExtension, ensurePageTaskAttrs, getPageTasksSnapshot) after those external imports so local relative imports are last and import ordering follows the project's guideline.packages/editor/src/extensions/task-metadata/TaskMentionExtension.ts (1)
1-40: Apply guideline conformance for new task-mention module declarations.Please add the package-level JSDoc banner, rename
TaskMentionSuggestionPluginKeyto SCREAMING_SNAKE_CASE, and convertTaskMentionOptionsto atypealias.As per coding guidelines, "Include file-level JSDoc at top of file:
/**@xnetjs/sync- Unified sync primitives for xNet */", "Constants must use SCREAMING_SNAKE_CASE:MAX_UPDATE_SIZE,DEFAULT_TTL", and "Prefertypeoverinterfacefor defining object shapes in TypeScript".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/editor/src/extensions/task-metadata/TaskMentionExtension.ts` around lines 1 - 40, Add the required file-level JSDoc banner at the very top of the file (use /** `@xnetjs/sync` - Unified sync primitives for xNet */), rename the PluginKey constant TaskMentionSuggestionPluginKey to a SCREAMING_SNAKE_CASE identifier (e.g., TASK_MENTION_SUGGESTION_PLUGIN_KEY) and update all references, and convert the exported TaskMentionOptions interface into an equivalent type alias named TaskMentionOptions while preserving its shape (getSuggestions and HTMLAttributes) and exports.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/src/routes/doc`.$docId.tsx:
- Line 101: The forEach callback on presence uses an expression-bodied arrow
which triggers the lint rule; change the callback to a block-bodied function so
the return value is explicit: locate the call presence.forEach((user) =>
addSuggestion(user)) and replace it with a block-bodied callback that calls
addSuggestion(user) inside a { ... } body (i.e., presence.forEach((user) => {
addSuggestion(user); })), keeping the same behavior and referencing
addSuggestion and presence.
In `@docs/explorations/0102_`[_]_AFFINE_BLOCKSUITE_INTEGRATION_FEASIBILITY.md:
- Line 810: The table entries claiming "BlockSuite is MPL 2.0 (permissive)" and
"MPL 2.0 → MIT is compatible" are inaccurate; update those cells (the row titled
"Legal issues - License or trademark concerns" and the two text snippets) to
either (a) replace the first with wording that MPL-2.0 is a weak copyleft
license requiring modified MPL-covered files distributed to remain under MPL-2.0
and source for those files be made available to recipients, and (b) replace the
second with wording that MIT and MPL-2.0 can be combined in a Larger Work but
MPL-covered files cannot be relicensed to MIT (or alternatively add "requires
legal review for specific distribution scenario" if you prefer a cautionary
note); ensure the revised text explicitly mentions the file-level obligations
and the correct compatibility constraint.
In `@packages/editor/src/components/FloatingToolbar.tsx`:
- Around line 119-123: Wrap the call to input.showPicker() inside a try-catch in
the handlePickDueDate flow: if typeof input.showPicker === 'function' attempt
await input.showPicker() inside try, on any caught error call input.click() as a
fallback, and ensure a final safety net by calling finish(null) in a finally
block (or after catch) so the promise cannot hang; retain the existing
input.click() branch for non-supporting browsers.
In `@packages/editor/src/components/RichTextEditor.tsx`:
- Around line 495-497: The initial call to publishPageTasks() happens before the
update listener is registered, so synchronous attribute updates from
ensurePageTaskAttrs(editor) trigger an 'update' event that is missed; swap the
order so you register the listener first by calling editor.on('update',
publishPageTasks) before invoking publishPageTasks(), ensuring publishPageTasks,
ensurePageTaskAttrs(editor) and the onPageTasksChange flow run with the listener
in place.
In `@packages/editor/src/components/TaskMentionMenu.tsx`:
- Around line 38-60: The onKeyDown handler exposed via useImperativeHandle must
guard against an empty items array to avoid NaN from modulo by zero: inside the
onKeyDown implementation (the function returned by useImperativeHandle that
handles 'ArrowUp', 'ArrowDown', and 'Enter'), check if items.length === 0 and
return false (or ignore keys) before performing modulo operations or calling
setSelectedIndex/selectItem; update the branches handling 'ArrowUp' and
'ArrowDown' to early-return when items is empty and ensure the 'Enter' branch
also checks items.length>0 before calling selectItem(selectedIndex).
In `@packages/editor/src/extensions/task-metadata/TaskDueDateExtension.ts`:
- Around line 163-176: The setTaskDueDate command currently creates a
taskDueDate node even when the selection is not inside a taskItem; update the
command (the ({ editor, state, dispatch }) => { ... } handler that calls
normalizeDateString, create on this.type, and uses
findCurrentTaskItem/findDueDateNode) to early-return false if
findCurrentTaskItem(editor) is null (i.e., cursor not inside a taskItem) before
creating or inserting nodes; also add a file-level JSDoc header at the top of
TaskDueDateExtension.ts and change the TaskDueDateOptions declaration from an
interface to a type alias to match project style.
In `@packages/react/src/hooks/usePageTaskSync.ts`:
- Around line 194-213: During mapping in usePageTaskSync, don't populate
referenceUpserts unconditionally; instead build a temporary nextReferences array
of ids while computing ids with computeExternalReferenceId and collecting the
reference objects, then after change detection using
arraysEqual(existingTask.references, referenceIds) (or the inverse check), only
transfer the collected upserts into referenceUpserts when references actually
differ; update the code paths that currently push into referenceUpserts inside
the map to push into a local collector and perform the push to referenceUpserts
after the no-op check.
- Around line 171-345: Introduce a run-id check to avoid stale DB mutations: at
the start of run() capture a local runId by incrementing/reading
syncRunIdRef.current (create syncRunIdRef if missing), and before each database
operation loop (the restore loop, tasksToCreate loop using
create(TaskSchema,...), taskUpdates loop using update(TaskSchema,...),
taskDeletes loop using remove(...), and referenceUpserts loop using
update/create on ExternalReferenceSchema) verify if cancelled || runId !==
syncRunIdRef.current and return early if true; keep existing cancelled checks
around setSyncing/setError but ensure DB operations bail when stale so stale
mutations cannot execute.
- Around line 106-112: The toDateTimestamp function uses local-time Date
construction which can auto-roll invalid calendar dates and yield
timezone-dependent results; update toDateTimestamp to parse the YYYY-MM-DD
parts, construct a UTC timestamp via Date.UTC(year, month-1, day), then verify
the constructed UTC date components match the parsed year/month/day to reject
overflow values (e.g., 2026-02-31) and return undefined on mismatch or NaN,
ensuring identical UTC timestamps across environments.
---
Nitpick comments:
In `@apps/electron/src/renderer/components/PageView.tsx`:
- Around line 105-129: The Biome lint warning comes from using presence.forEach
with addSuggestion which has an early return; change the iteration to a for...of
loop to avoid returning from a forEach callback. In the memo block around
mentionSuggestions, replace the presence.forEach(...) call with a simple for
(const user of presence) call that invokes addSuggestion(user) (keep
addSuggestion, presence, did and mentionSuggestions names intact) so behavior is
unchanged but the lint warning is eliminated.
In
`@docs/explorations/0103_`[_]_TASKS_EMBEDDED_IN_PAGES_BACKED_BY_NODES_MENTIONS_DUE_DATES_NESTED_SUBTASKS_DATABASES_CANVASES_AND_CROSS_SURFACE_TASK_MODEL.md:
- Line 220: The markdown has an extra space inside the code span around the
square brackets — update the span containing `[] ` so it has no internal
trailing/leading spaces (change `[] ` to `[]`) where the fragment reads "type
`[] `," to satisfy the linter; search for the code span `[] ` and remove the
space so it becomes `[]`.
In `@packages/editor/src/components/RichTextEditor.tsx`:
- Around line 4-44: Move all external/package imports to the top and place the
local/internal imports (everything imported from '../extensions') after them:
keep imports from '@tiptap/*', 'y-protocols/awareness', 'yjs', 'react', etc. at
the top, then import the types and symbols coming from '../extensions'
(DatabaseViewType, PageTaskSnapshot, SlashCommandItem, TaskMentionSuggestion,
Wikilink, LivePreview, HeadingWithSyntax, CodeBlockWithSyntax,
BlockquoteWithSyntax, SlashCommand, DragHandleExtension,
KeyboardShortcutsExtension, ImageExtension, CalloutExtension, ToggleExtension,
FileExtension, SmartReferenceExtension, EmbedExtension, DatabaseEmbedExtension,
PageTaskItemExtension, TaskMentionExtension, TaskDueDateExtension,
ensurePageTaskAttrs, getPageTasksSnapshot) after those external imports so local
relative imports are last and import ordering follows the project's guideline.
In `@packages/editor/src/components/TaskMentionMenu.tsx`:
- Around line 4-18: Replace the exported and local interfaces with type aliases:
change TaskMentionSuggestion, TaskMentionMenuProps, and TaskMentionMenuRef from
interface declarations to equivalent type declarations so the object shapes use
type instead of interface; ensure the exported name TaskMentionSuggestion
remains exported and that the signatures for command and onKeyDown keep the same
types.
- Around line 1-3: Add a file-level JSDoc comment at the top of
TaskMentionMenu.tsx describing the module's purpose and exported component(s)
(e.g., that this file implements the TaskMentionMenu React component used for
inserting task mentions), including any relevant tags like `@module` and a brief
summary; place the JSDoc immediately above the existing imports so it applies to
the whole file and keep it concise per the repository guidelines.
In `@packages/editor/src/extensions/page-tasks/index.ts`:
- Around line 1-27: Add the required file-level JSDoc header exactly as /**
`@xnetjs/sync` - Unified sync primitives for xNet */ at the very top of the file,
and convert the exported interfaces PageTaskReferenceSnapshot and
PageTaskSnapshot into type aliases with identical object shapes (replace "export
interface PageTaskReferenceSnapshot { ... }" and "export interface
PageTaskSnapshot { ... }" with "export type PageTaskReferenceSnapshot = { ... }"
and "export type PageTaskSnapshot = { ... }" respectively), keeping all property
names and types unchanged.
In `@packages/editor/src/extensions/smart-reference/providers.ts`:
- Around line 21-31: Replace the exported interface SmartReference with a type
alias that preserves the exact object shape; change the declaration from "export
interface SmartReference { ... }" to "export type SmartReference = { ... }"
keeping all fields (provider, kind: SmartReferenceKind, url, refId, title,
subtitle?, icon, embedUrl?, metadata: Record<string,string>) and export, and
then run a quick grep for any places that rely on interface-specific features
(like declaration merging or "implements") and adjust those usages if necessary.
In
`@packages/editor/src/extensions/smart-reference/SmartReferenceExtension.test.ts`:
- Line 1: Add the required file-level JSDoc banner at the very top of the test
file before any imports; insert the exact header /** `@xnetjs/sync` - Unified sync
primitives for xNet */ above the existing import statement (the file currently
imports Editor from '@tiptap/core') so the file complies with the
packages/**/*.{ts,tsx} guideline and the package-level standard.
In `@packages/editor/src/extensions/smart-reference/SmartReferenceExtension.ts`:
- Around line 11-47: Rename the PluginKey constant SmartReferencePastePluginKey
to SCREAMING_SNAKE_CASE (e.g., SMART_REFERENCE_PASTE_PLUGIN_KEY) and update
every reference to that identifier (including where new
PluginKey('smartReferencePaste') is created and any uses/imports/exports). Also
convert the exported interface SmartReferenceOptions into a type alias (e.g.,
type SmartReferenceOptions = { autoConvertTaskPaste: boolean; HTMLAttributes:
Record<string, string>; }) and update any places that reference the interface to
use the new type alias. Ensure exports and any imports elsewhere are updated to
the new names to keep types and runtime identifiers consistent.
In `@packages/editor/src/extensions/task-metadata/TaskDueDateExtension.ts`:
- Around line 1-4: Add the required file-level JSDoc header at the top of
TaskDueDateExtension.ts (e.g. /** `@xnetjs/sync` - Unified sync primitives for
xNet */) and replace any exported interface used for options (e.g.
TaskDueDateOptions or similar options interface/type declarations) with a
TypeScript `type` alias instead of `interface`; update any references to that
symbol (constructor signatures, defaultOptions, getOptions, or extension
declaration) to use the new type name so the file follows the coding guideline
preferring `type` for object shapes.
In `@packages/editor/src/extensions/task-metadata/TaskMentionExtension.ts`:
- Around line 1-40: Add the required file-level JSDoc banner at the very top of
the file (use /** `@xnetjs/sync` - Unified sync primitives for xNet */), rename
the PluginKey constant TaskMentionSuggestionPluginKey to a SCREAMING_SNAKE_CASE
identifier (e.g., TASK_MENTION_SUGGESTION_PLUGIN_KEY) and update all references,
and convert the exported TaskMentionOptions interface into an equivalent type
alias named TaskMentionOptions while preserving its shape (getSuggestions and
HTMLAttributes) and exports.
In `@packages/react/src/hooks/usePageTaskSync.test.tsx`:
- Line 1: Add the repository’s required file-level JSDoc banner at the very top
of the file before the first import (currently "import type { DID } from
'@xnetjs/core'"); insert the exact header /** `@xnetjs/sync` - Unified sync
primitives for xNet */ as the first line so the file complies with the
packages/**/*.{ts,tsx} guideline.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 55e28434-55e2-4538-9e3c-03054525c325
📒 Files selected for processing (34)
apps/electron/src/renderer/components/PageView.tsxapps/web/src/components/Editor.tsxapps/web/src/routes/doc.$docId.tsxdocs/explorations/0102_[_]_AFFINE_BLOCKSUITE_INTEGRATION_FEASIBILITY.mddocs/explorations/0103_[_]_TASKS_EMBEDDED_IN_PAGES_BACKED_BY_NODES_MENTIONS_DUE_DATES_NESTED_SUBTASKS_DATABASES_CANVASES_AND_CROSS_SURFACE_TASK_MODEL.mdpackages/data/src/index.tspackages/data/src/schema/index.tspackages/data/src/schema/schemas/external-reference.test.tspackages/data/src/schema/schemas/external-reference.tspackages/data/src/schema/schemas/index.tspackages/data/src/schema/schemas/task.test.tspackages/data/src/schema/schemas/task.tspackages/editor/src/components/FloatingToolbar.tsxpackages/editor/src/components/RichTextEditor.tsxpackages/editor/src/components/TaskMentionMenu.tsxpackages/editor/src/components/editor-ux-state.tspackages/editor/src/extensions.tspackages/editor/src/extensions/page-tasks/PageTaskItemExtension.test.tspackages/editor/src/extensions/page-tasks/index.tspackages/editor/src/extensions/smart-reference/SmartReferenceExtension.test.tspackages/editor/src/extensions/smart-reference/SmartReferenceExtension.tspackages/editor/src/extensions/smart-reference/index.tspackages/editor/src/extensions/smart-reference/providers.test.tspackages/editor/src/extensions/smart-reference/providers.tspackages/editor/src/extensions/task-metadata/TaskDueDateExtension.tspackages/editor/src/extensions/task-metadata/TaskMentionExtension.tspackages/editor/src/extensions/task-metadata/TaskMetadataExtension.test.tspackages/editor/src/extensions/task-metadata/index.tspackages/editor/src/react.tspackages/editor/src/styles/editor.csspackages/react/src/hooks/usePageTaskSync.test.tsxpackages/react/src/hooks/usePageTaskSync.tspackages/react/src/index.tssite/src/layouts/Base.astro
| if (typeof input.showPicker === 'function') { | ||
| input.showPicker() | ||
| } else { | ||
| input.click() | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
wc -l packages/editor/src/components/FloatingToolbar.tsxRepository: crs48/xNet
Length of output: 108
🏁 Script executed:
sed -n '100,150p' packages/editor/src/components/FloatingToolbar.tsxRepository: crs48/xNet
Length of output: 1161
🏁 Script executed:
sed -n '1,30p' packages/editor/src/components/FloatingToolbar.tsxRepository: crs48/xNet
Length of output: 1026
🏁 Script executed:
sed -n '80,130p' packages/editor/src/components/FloatingToolbar.tsxRepository: crs48/xNet
Length of output: 1127
🏁 Script executed:
sed -n '60,120p' packages/editor/src/components/FloatingToolbar.tsxRepository: crs48/xNet
Length of output: 1492
🌐 Web query:
HTMLInputElement showPicker() throws error browser compatibility
💡 Result:
HTMLInputElement.showPicker() errors usually come from one of these causes:
-
Browser doesn’t support it (or is too old)
If you seeTypeError: input.showPicker is not a function, that browser/version doesn’t implement it. Support starts around Chrome/Edge 99+, Firefox 101+, Safari 16+ (macOS + iOS). Check the exact versions your users have. [2] -
Not triggered by a user gesture
If you call it fromsetTimeout,Promise.then, page load, etc., browsers can throwNotAllowedErrorbecause it requires transient user activation (e.g., inside a click/tap handler). [1] [3] -
Cross-origin iframe restrictions
If the input is inside a cross-origin<iframe>, it can throwSecurityError(with exceptions fortype="file"andtype="color"). [1] [3] -
Input is not “mutable”
If the input is not user-editable (commonlydisabled, and in some cases other states), it may throwInvalidStateError. [1] [3] -
Safari/iOS reality check (partial/buggy behavior)
Even where “supported”, there are real-world Safari/iOS limitations/bugs—e.g., reports that on iOS/iPadOS it may effectively work only for certain picker types, and picker-closing behavior can be problematic. Treat it as progressive enhancement. [4]
Recommended safe pattern
button.addEventListener("click", () => {
const input = document.querySelector('input[type="date"]');
if (typeof input?.showPicker === "function") {
try {
input.showPicker(); // must be in this user-gesture handler
} catch (e) {
input.focus(); // fallback (may open picker on some mobile browsers)
}
} else {
input.focus(); // fallback for unsupported browsers
}
});If your “error” is TypeScript (“Property 'showPicker' does not exist…”)
That’s a TypeScript DOM typings version issue: update TypeScript (the DOM lib typings come from TS), or temporarily cast: (input as any).showPicker?.(). [5]
🌐 Web query:
showPicker() NotAllowedError NotSupportedError browser states
💡 Result:
showPicker() can fail with different DOMException names depending on state/context:
-
NotAllowedError: thrown when the call is not made with transient user activation (i.e., not directly in a trusted user gesture like a click/tap/keydown handler). This commonly happens after anawait, insidesetTimeout, promise callbacks, or message handlers where activation is no longer “transient.” [1][2] -
NotSupportedError: (per the HTML Standard) applies toHTMLSelectElement.showPicker()when the<select>is not being rendered (e.g., not rendered due to CSS/layout). This is not the general “browser doesn’t support showPicker” case. [1]
Related (often confused with “unsupported”):
-
No exception; just does nothing: if an
<input>doesn’t support a picker,HTMLInputElement.showPicker()is specified to do nothing (noNotSupportedError). [1] -
InvalidStateError: if the element is not mutable (e.g.,disabled/readonlyin contexts where it makes it non-mutable). [1][2] -
SecurityError: if called in a cross-origin iframe, except fortype="file"andtype="color"which are exempt. [1][2]
Sources:
[1] HTML Standard (showPicker() algorithm and exceptions) (html.spec.whatwg.org)
[2] MDN: HTMLInputElement.showPicker() exceptions/security notes (developer.mozilla.org)
Citations:
- 1: https://html.spec.whatwg.org/multipage/input.html
- 2: https://developer.mozilla.org/en-US/docs/Web/API/HTMLInputElement/showPicker
🏁 Script executed:
# Check if pickDate is called from event handlers to understand activation context
rg -A 3 "pickDate\(" packages/editor/src/components/FloatingToolbar.tsxRepository: crs48/xNet
Length of output: 403
🏁 Script executed:
# Verify if there's any timeout/cleanup logic in the pickDate promise
sed -n '60,150p' packages/editor/src/components/FloatingToolbar.tsx | rg -A 5 "new Promise"Repository: crs48/xNet
Length of output: 265
🏁 Script executed:
# Check if there's any timeout or promise rejection handling in pickDate or its callers
sed -n '220,260p' packages/editor/src/components/FloatingToolbar.tsxRepository: crs48/xNet
Length of output: 1274
🏁 Script executed:
# Check if there's any error handling or timeout wrapper around the pickDate call
sed -n '230,250p' packages/editor/src/components/FloatingToolbar.tsxRepository: crs48/xNet
Length of output: 662
Guard showPicker() with a try-catch to prevent promise hangs and add fallback behavior.
Lines 119-123 call showPicker() unguarded. This Web API can throw NotAllowedError, InvalidStateError, or SecurityError in various browser states/contexts. If an exception occurs, the promise never resolves because no event listeners catch it, causing handlePickDueDate to hang indefinitely. Add a try-catch wrapper with fallback to input.click() and a final finish(null) safety net.
🛠️ Suggested resilience patch
- if (typeof input.showPicker === 'function') {
- input.showPicker()
- } else {
- input.click()
- }
+ try {
+ if (typeof input.showPicker === 'function') {
+ input.showPicker()
+ } else {
+ input.click()
+ }
+ } catch {
+ try {
+ input.click()
+ } catch {
+ finish(null)
+ }
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (typeof input.showPicker === 'function') { | |
| input.showPicker() | |
| } else { | |
| input.click() | |
| } | |
| try { | |
| if (typeof input.showPicker === 'function') { | |
| input.showPicker() | |
| } else { | |
| input.click() | |
| } | |
| } catch { | |
| try { | |
| input.click() | |
| } catch { | |
| finish(null) | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/editor/src/components/FloatingToolbar.tsx` around lines 119 - 123,
Wrap the call to input.showPicker() inside a try-catch in the handlePickDueDate
flow: if typeof input.showPicker === 'function' attempt await input.showPicker()
inside try, on any caught error call input.click() as a fallback, and ensure a
final safety net by calling finish(null) in a finally block (or after catch) so
the promise cannot hang; retain the existing input.click() branch for
non-supporting browsers.
|
Preview: https://xnet.fyi/pr/4/app/ |
| if: github.event.pull_request.head.repo.full_name == github.repository | ||
| runs-on: ubuntu-latest | ||
| steps: |
There was a problem hiding this comment.
Shared concurrency group cancels cross-workflow deployments
All four workflows (deploy-pr-preview.yml, deploy-site.yml, remove-pr-preview.yml, undeploy-site.yml) share group: pages-branch with cancel-in-progress: true. This means a push to main will cancel any in-flight PR preview deployment (and vice versa).
In the worst case: a PR preview run is mid-way through the rsync + git add steps when a main push triggers the site deploy and cancels it. The gh-pages worktree at /tmp/xnet-gh-pages is abandoned mid-edit, but since the push hadn't happened yet, the remote gh-pages branch is unaffected. The PR preview simply never gets published, with no error surfaced to the PR author.
A safer pattern is to use separate concurrency groups for PR previews and site deploys — they both operate on gh-pages but write to distinct subdirectories (/pr/<N>/ vs everything else):
# In deploy-pr-preview.yml / remove-pr-preview.yml
concurrency:
group: pages-pr-${{ github.event.pull_request.number }}
cancel-in-progress: true
# In deploy-site.yml / undeploy-site.yml
concurrency:
group: pages-site
cancel-in-progress: trueThe rsync --exclude pr in deploy-site.yml and rsync -a --delete inside /pr/<N>/ in the preview workflows already keep them isolated at the file level, so separate groups would not introduce write conflicts.
This same issue is present in .github/workflows/remove-pr-preview.yml (line 17) and .github/workflows/undeploy-site.yml (line 12).
| workflow_dispatch: | ||
|
|
||
| permissions: |
There was a problem hiding this comment.
contents: write is broader than the minimum required permission
The previous implementation used pages: write + id-token: write, which are scoped specifically to GitHub Pages deployments via the OIDC token exchange. The new approach replaces these with contents: write, which grants the workflow token write access to every branch and file in the repository.
For the site deploy (triggered by pushes to main), this is only exploitable by someone who can merge to main — acceptable for this repo. However, deploy-pr-preview.yml also uses contents: write, and it's triggered by pull_request events. Although the fork guard (if: github.event.pull_request.head.repo.full_name == github.repository) prevents external-fork PRs from running the job, any internal collaborator with PR-creation rights could run arbitrary workflow steps with write-to-all-branches access.
Consider splitting the token scope more tightly, or documenting the accepted risk:
permissions:
contents: write # Required to push to gh-pages branch only
pull-requests: write # Required to post preview link commentsGitHub doesn't yet offer a gh-pages: write fine-grained permission, so the comment-based documentation is the pragmatic mitigation here.
| export interface PageTaskReferenceInput { | ||
| url: string | ||
| provider: string | null | ||
| kind: string | null | ||
| refId: string | null | ||
| title: string | null | ||
| subtitle: string | null | ||
| icon: string | null | ||
| embedUrl: string | null | ||
| metadata: string | ||
| } | ||
|
|
||
| export interface PageTaskInput { | ||
| taskId: string | ||
| blockId: string | ||
| title: string | ||
| completed: boolean | ||
| parentTaskId: string | null | ||
| sortKey: string | ||
| assignees: string[] | ||
| dueDate: string | null | ||
| references: PageTaskReferenceInput[] | ||
| } | ||
|
|
||
| export interface UsePageTaskSyncOptions { | ||
| pageId: string | null | ||
| debounceMs?: number | ||
| } | ||
|
|
||
| export interface UsePageTaskSyncResult { | ||
| handleTasksChange: (tasks: PageTaskInput[]) => void | ||
| syncing: boolean | ||
| error: Error | null | ||
| } |
There was a problem hiding this comment.
interface used instead of type (AGENTS.md style guide)
AGENTS.md states: "Prefer type over interface for object shapes." The four public-API shapes added in this file all use interface:
export interface PageTaskReferenceInput(line 13)export interface PageTaskInput(line 25)export interface UsePageTaskSyncOptions(line 37)export interface UsePageTaskSyncResult(line 42)
The same pattern appears throughout the PR in several other new files:
packages/editor/src/components/TaskMentionMenu.tsx—export interface TaskMentionSuggestion(~line 4)packages/editor/src/extensions/smart-reference/providers.ts—export interface SmartReference(~line 25)packages/editor/src/extensions/smart-reference/SmartReferenceExtension.ts—export interface SmartReferenceOptions(~line 34)packages/editor/src/extensions/task-metadata/TaskMentionExtension.ts—export interface TaskMentionOptions(~line 40)packages/editor/src/extensions/task-metadata/TaskDueDateExtension.ts—export interface TaskDueDateOptions(~line 73)
All of these should be export type declarations instead:
export type PageTaskReferenceInput = {
url: string
provider: string | null
// ...
}Context Used: Rule from dashboard - AGENTS.md (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/deploy-site.yml (1)
63-90: Consider extracting the gh-pages git/worktree publish block into a shared action.The same publish sequence is repeated across multiple workflows; centralizing it would reduce drift and simplify future fixes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/deploy-site.yml around lines 63 - 90, Extract the repeated "Publish production site to gh-pages" run block (the sequence that configures git user, creates/updates the /tmp/xnet-gh-pages worktree with git worktree add -B gh-pages, syncs files with rsync, stages changes, commits and pushes) into a reusable GitHub Action; create an action (e.g., actions/publish-gh-pages) that accepts inputs for source directory (default /tmp/xnet-pages-root), target branch (gh-pages), commit message, and optional exclusions, implement the same steps (git config, fetch, conditional git worktree add, rsync --delete --exclude, git add/commit/push) inside the action, and replace this run block in the workflow with a single uses: ./actions/publish-gh-pages (or the published action) invocation passing those inputs so all workflows reuse the centralized logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/deploy-site.yml:
- Around line 18-19: Multiple deployment workflows share the same
concurrency.group (group: pages-branch) and currently set cancel-in-progress:
true, which lets newer preview/cleanup runs cancel an in-flight production
deploy; update the concurrency block in deploy-site.yml, deploy-pr-preview.yml,
remove-pr-preview.yml, and undeploy-site.yml to set cancel-in-progress: false
(i.e., concurrency: { group: pages-branch, cancel-in-progress: false }) or
alternatively switch to unique concurrency groups (e.g., group: ${{
github.workflow }}-${{ github.ref }}) to prevent cross-workflow cancellations.
---
Nitpick comments:
In @.github/workflows/deploy-site.yml:
- Around line 63-90: Extract the repeated "Publish production site to gh-pages"
run block (the sequence that configures git user, creates/updates the
/tmp/xnet-gh-pages worktree with git worktree add -B gh-pages, syncs files with
rsync, stages changes, commits and pushes) into a reusable GitHub Action; create
an action (e.g., actions/publish-gh-pages) that accepts inputs for source
directory (default /tmp/xnet-pages-root), target branch (gh-pages), commit
message, and optional exclusions, implement the same steps (git config, fetch,
conditional git worktree add, rsync --delete --exclude, git add/commit/push)
inside the action, and replace this run block in the workflow with a single
uses: ./actions/publish-gh-pages (or the published action) invocation passing
those inputs so all workflows reuse the centralized logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2b3d3508-b233-468b-94f4-b48776087b5a
📒 Files selected for processing (4)
.github/workflows/deploy-pr-preview.yml.github/workflows/deploy-site.yml.github/workflows/remove-pr-preview.yml.github/workflows/undeploy-site.yml
| group: pages-branch | ||
| cancel-in-progress: true |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
GitHub Actions cancel-in-progress shared concurrency group behavior multiple workflows
💡 Result:
In GitHub Actions, concurrency groups are scoped at the repository level, so different workflows share the same concurrency “slot” if they use the same concurrency.group value—and they can block/cancel each other. [1]
What happens when multiple workflows share a group
For a given concurrency group, GitHub ensures at most 1 running and 1 pending run/job at a time. If a new run is queued into the same group while another is running, the new one becomes pending, and any existing pending run in that group is canceled and replaced by the newer one. [1]
How cancel-in-progress interacts across workflows
- If the newly-triggered run has
cancel-in-progress: true, GitHub will cancel the currently running run/job in that same group—even if it’s from a different workflow. [1] - If the newly-triggered run has
cancel-in-progress: false(or omitted), it won’t cancel the running one; it will just wait inpending(still replacing any older pending run). [1]
Avoiding cross-workflow cancellation
GitHub explicitly warns that if you have multiple workflows, concurrency group names must be unique across workflows to avoid canceling each other. A common pattern is:
group: ${{ github.workflow }}-${{ github.ref }}. [1]
Sources: GitHub Docs on workflow/job concurrency and cross-workflow group collisions/cancellation behavior. [1]
🏁 Script executed:
# Find and examine the workflow files mentioned
fd -t f "deploy-site.yml|deploy-pr-preview.yml|remove-pr-preview.yml|undeploy-site.yml" .github/workflows/Repository: crs48/xNet
Length of output: 203
🏁 Script executed:
# Check deploy-site.yml concurrency section
head -n 25 .github/workflows/deploy-site.yml | tail -n 10Repository: crs48/xNet
Length of output: 205
🏁 Script executed:
# Check all four workflows for their concurrency groups
for file in .github/workflows/deploy-site.yml .github/workflows/deploy-pr-preview.yml .github/workflows/remove-pr-preview.yml .github/workflows/undeploy-site.yml; do
echo "=== $file ==="
grep -A 5 "^concurrency:" "$file" || echo "No concurrency found"
doneRepository: crs48/xNet
Length of output: 569
Remove cancel-in-progress: true from shared concurrency group to prevent production deploys from being canceled.
When multiple workflows share the same concurrency.group, setting cancel-in-progress: true allows any newly-triggered workflow to cancel currently running jobs from other workflows in that group. With all four deployment workflows (deploy-site.yml, deploy-pr-preview.yml, remove-pr-preview.yml, undeploy-site.yml) using group: pages-branch, a preview or cleanup run can cancel an in-flight production deploy, leaving gh-pages stale.
Set cancel-in-progress: false in all four workflows:
Changes needed
.github/workflows/deploy-site.yml(line 19).github/workflows/deploy-pr-preview.yml.github/workflows/remove-pr-preview.yml.github/workflows/undeploy-site.yml
Change from cancel-in-progress: true to cancel-in-progress: false in each.
Alternatively, use unique concurrency groups per workflow (e.g., group: ${{ github.workflow }}-${{ github.ref }}) to prevent cross-workflow interference.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/deploy-site.yml around lines 18 - 19, Multiple deployment
workflows share the same concurrency.group (group: pages-branch) and currently
set cancel-in-progress: true, which lets newer preview/cleanup runs cancel an
in-flight production deploy; update the concurrency block in deploy-site.yml,
deploy-pr-preview.yml, remove-pr-preview.yml, and undeploy-site.yml to set
cancel-in-progress: false (i.e., concurrency: { group: pages-branch,
cancel-in-progress: false }) or alternatively switch to unique concurrency
groups (e.g., group: ${{ github.workflow }}-${{ github.ref }}) to prevent
cross-workflow cancellations.
- add a useTasks hook for page and assignee task queries - fix page task sync UTC due dates and reference write order - add My Tasks and page task views in web and page task view in Electron - extend hook coverage for subscribed task view updates - check off the completed exploration items
- add a taskViewEmbed editor block with persisted scope, assignee, status, due-date, and hierarchy filters - wire embedded task rendering through the shared editor surface in web and desktop page views - extend task queries with due-date presets and update the exploration checklist for the shipped slice
- add gravatar-backed avatars to task mention suggestions and the autocomplete menu - include the current user in page mention suggestions on web and desktop - cover the gravatar helper and mention menu avatar rendering with focused tests
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (2)
packages/editor/src/components/RichTextEditor.tsx (1)
515-516:⚠️ Potential issue | 🟠 MajorRegister the
updatelistener before the initial publish.Line 515 runs
publishPageTasks()before Line 516 registers the listener, so an update emitted duringensurePageTaskAttrs(editor)can be dropped.Suggested fix
- publishPageTasks() - editor.on('update', publishPageTasks) + editor.on('update', publishPageTasks) + publishPageTasks()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/editor/src/components/RichTextEditor.tsx` around lines 515 - 516, The initial call to publishPageTasks() runs before registering the editor.update listener, so updates emitted during ensurePageTaskAttrs(editor) can be missed; fix by registering the listener first (call editor.on('update', publishPageTasks) before calling publishPageTasks()) so that any update fired during ensurePageTaskAttrs(editor) will be handled by the publishPageTasks handler.apps/web/src/routes/doc.$docId.tsx (1)
102-102:⚠️ Potential issue | 🟡 MinorUse a block-bodied
forEachcallback to satisfy Biome.Line 102 still triggers
lint/suspicious/useIterableCallbackReturn.🔧 Suggested fix
- presence.forEach((user) => addSuggestion(user)) + presence.forEach((user) => { + addSuggestion(user) + })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/routes/doc`.$docId.tsx at line 102, The one-line arrow callback used with presence.forEach is flagged by the linter; change the callback to a block-bodied form so the forEach invocation becomes a callback that calls addSuggestion(user) inside a block (e.g., presence.forEach((user) => { addSuggestion(user); })), ensuring the symbol presence.forEach and the function addSuggestion are used unchanged but the callback has a block body to satisfy Biome's lint rule.
🧹 Nitpick comments (12)
packages/editor/src/components/RichTextEditor.tsx (1)
4-14: Reorder type imports to match repository import-order rules.Lines 4-14 currently place a local relative type import before external type imports. Move external type-only imports first, and keep local relative imports last.
As per coding guidelines: “Type-only imports must come first”, “Internal package imports should follow external packages”, and “Local relative imports should come last”.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/editor/src/components/RichTextEditor.tsx` around lines 4 - 14, Reorder the type imports in RichTextEditor.tsx so external type-only imports come first and the local relative import comes last: move the imports from '@tiptap/core' (AnyExtension), 'y-protocols/awareness' (Awareness), and 'yjs' (Y) to appear before the local '../extensions' type import (DatabaseViewType, PageTaskSnapshot, SlashCommandItem, TaskMentionSuggestion, TaskViewConfig, TaskViewEmbedType) so the module order follows the repository rules (external packages, then internal packages, then local relative).apps/web/src/components/PageTasksPanel.tsx (1)
5-7: Use atypealias for props to match project convention.Convert
PageTasksPanelPropsfrominterfacetotypefor consistency with the repo rule.Suggested fix
-interface PageTasksPanelProps { +type PageTasksPanelProps = { pageId: string -} +}As per coding guidelines: “Prefer
typeoverinterfacefor defining object shapes in TypeScript”.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/PageTasksPanel.tsx` around lines 5 - 7, Replace the PageTasksPanelProps interface with a type alias to follow the project convention: change the declaration named PageTasksPanelProps from an `interface` to a `type` (e.g., `type PageTasksPanelProps = { pageId: string }`) and ensure any usages inside the PageTasksPanel component still refer to PageTasksPanelProps without other changes.packages/editor/src/extensions/task-view-embed/index.ts (1)
1-1: Add the required package file-level JSDoc header.This new
packages/*file starts directly with exports; add the required top-of-file JSDoc banner.As per coding guidelines "packages/**/*.{ts,tsx}: Include file-level JSDoc at top of file".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/editor/src/extensions/task-view-embed/index.ts` at line 1, This file is missing the required top-of-file JSDoc banner; add a file-level JSDoc comment at the very top of the file (above the existing export line) describing the module and any relevant metadata per project guidelines so that the file exporting TaskViewEmbedExtension and DEFAULT_TASK_VIEW_CONFIG includes the mandated header; ensure the comment follows the same JSDoc format used across packages (brief description, optional `@module/`@file or author/license tags if used in other files).apps/web/src/components/Editor.tsx (1)
42-53: Add an explicit return type for the exportedEditorcomponent.Please annotate
Editorwith an explicit JSX return type for consistency with repository TS rules.As per coding guidelines "Use explicit return types on exported functions".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/Editor.tsx` around lines 42 - 53, The exported Editor function lacks an explicit return type; update its signature (the export function Editor(...) declaration) to include a JSX return type (e.g., ): JSX.Element or React.JSX.Element) so the exported component has an explicit return type per project rules; adjust any imports if your chosen annotation requires them and ensure the Props type remains unchanged.packages/editor/src/extensions/task-view-embed/TaskViewEmbedNodeView.tsx (1)
1-190: Add required package-level documentation scaffolding.This new
packages/editorfile is missing the required file-level JSDoc and section/doc conventions for exported functionality.As per coding guidelines "packages/**/*.{ts,tsx}: Include file-level JSDoc at top of file", "Use section dividers in code", and "Function documentation should include JSDoc with
@exampletags".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/editor/src/extensions/task-view-embed/TaskViewEmbedNodeView.tsx` around lines 1 - 190, Add the required file-level JSDoc and section dividers and add JSDoc for exported items: insert a top-of-file JSDoc block describing the package/editor extension file, its purpose and exports; add clear section dividers (e.g., “// Constants”, “// Helpers”, “// Components / NodeViews”) around SCOPE_OPTIONS/ASSIGNEE_OPTIONS/STATUS_OPTIONS/DUE_DATE_OPTIONS, getConfig and FilterSelect, and the main exported function; and add a JSDoc comment for the exported TaskViewEmbedNodeView function that documents parameters and includes an `@example` showing basic usage (call signature showing node props or usage with extension.options.renderView). Ensure the exported symbol name TaskViewEmbedNodeView and helper names getConfig and FilterSelect are referenced in the comments where appropriate.apps/web/src/components/MyTasksPanel.tsx (1)
1-5: Align import grouping and exported signature with TS guidelines.Please reorder imports so all external packages come before internal
@xnetjs/*imports, and add an explicit return type toMyTasksPanel.As per coding guidelines "Internal package imports should follow external packages" and "Use explicit return types on exported functions".
Also applies to: 20-20
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/MyTasksPanel.tsx` around lines 1 - 5, Reorder the import block so all external packages (e.g., '@tanstack/react-router', 'lucide-react', 'react' / 'useMemo, useState') appear before internal `@xnetjs/`* imports (move "PageSchema", "useIdentity", "useQuery", "useTasks" after external imports), and add an explicit return type to the exported component signature for MyTasksPanel (e.g., export function MyTasksPanel(...): JSX.Element { ... }) so the function has a clear typed return; apply the same import ordering and explicit return type adjustment to the other occurrence noted.apps/electron/src/renderer/components/PageTasksPanel.tsx (2)
5-7: Align component typing with repository TS conventions.Use a
typealias for props and add an explicit return type on the exported component.As per coding guidelines "Prefer
typeoverinterfacefor defining object shapes in TypeScript" and "Use explicit return types on exported functions".Also applies to: 22-22
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/electron/src/renderer/components/PageTasksPanel.tsx` around lines 5 - 7, Replace the interface PageTasksPanelProps with a type alias (e.g., type PageTasksPanelProps = { pageId: string }) and update the exported component declaration to include an explicit return type (e.g., export const PageTasksPanel = (props: PageTasksPanelProps): JSX.Element => { ... }); ensure you update any other occurrence noted (the similar definition at the other spot) to follow the same pattern so props use a type alias and the component has an explicit JSX.Element return type.
22-115: Extract this panel into shared code to prevent web/electron drift.This implementation mirrors
apps/web/src/components/PageTasksPanel.tsxline-for-line. Consider one shared component to avoid dual maintenance.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/electron/src/renderer/components/PageTasksPanel.tsx` around lines 22 - 115, The PageTasksPanel component is duplicated in web and electron; extract a single shared component to avoid drift by moving the implementation of PageTasksPanel (which uses useTasks, formatDueDate, isOverdue and icons like CheckSquare2, Square, Calendar, Users) into a shared UI package and have both apps import it; update the electron and web exports to import the shared PageTasksPanel and ensure any app-specific dependencies (styling classes or icon imports) are either re-exported from the shared package or injected via props (e.g., pass icons or className overrides) so the shared component compiles in both environments.packages/react/src/components/TaskCollectionEmbed.tsx (1)
1-12: Apply package TS conventions for new public props typing/file header.Please switch
TaskCollectionEmbedPropsto atypealias and add the required package file-level JSDoc header.As per coding guidelines "Prefer
typeoverinterfacefor defining object shapes in TypeScript" and "packages/**/*.{ts,tsx}: Include file-level JSDoc at top of file".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react/src/components/TaskCollectionEmbed.tsx` around lines 1 - 12, Replace the exported interface TaskCollectionEmbedProps with a type alias (e.g., "type TaskCollectionEmbedProps = { ... }") preserving the same property names and types, and add the required file-level JSDoc header at the very top of the TaskCollectionEmbed.tsx file (above imports) following package conventions; update any references to TaskCollectionEmbedProps if needed to ensure exports/imports remain correct.apps/electron/src/renderer/components/PageView.tsx (1)
107-131: Extract mention-suggestion building into a shared helper.This logic is duplicated with
apps/web/src/routes/doc.$docId.tsxand will drift. A shared utility (web/electron) would keep behavior consistent.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/electron/src/renderer/components/PageView.tsx` around lines 107 - 131, Extract the duplicate mention-suggestion logic into a shared helper (e.g., buildMentionSuggestions) and replace the inline useMemo in PageView with a call to that helper: move the Map creation, addSuggestion function, and final Array.from(suggestions.values()) into the new utility which accepts (presence, did, optional isLocal label/subtitle override) and returns the suggestions array; update PageView to import and call buildMentionSuggestions(presence, did) (and likewise update apps/web/src/routes/doc.$docId.tsx to use the same helper) so both locations reuse the same mentionSuggestions construction logic (referencing the existing symbols mentionSuggestions, addSuggestion, presence, did, and the useMemo wrapper).packages/react/src/hooks/useTasks.ts (1)
12-33: Usetypealiases for these object shapes to match repo standards.These exported shapes should use
typeinstead ofinterfacefor consistency with project conventions.As per coding guidelines, "Prefer `type` over `interface` for defining object shapes in TypeScript".Suggested refactor
-export interface UseTasksOptions { +export type UseTasksOptions = { pageId?: string | null assigneeDid?: string | null includeCompleted?: boolean statuses?: TaskStatus[] parentTaskId?: string | null dueDateFilter?: 'any' | 'overdue' | 'today' | 'next-7-days' | 'none' -} +} -export interface TaskTreeItem { +export type TaskTreeItem = { task: TaskNode depth: number children: TaskTreeItem[] -} +} -export interface UseTasksResult { +export type UseTasksResult = { data: TaskNode[] tree: TaskTreeItem[] loading: boolean error: Error | null reload: () => void -} +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react/src/hooks/useTasks.ts` around lines 12 - 33, Replace the exported interfaces UseTasksOptions, TaskTreeItem, and UseTasksResult with equivalent exported type aliases to match repo standards; keep the exact exported names and all properties (pageId, assigneeDid, includeCompleted, statuses, parentTaskId, dueDateFilter on UseTasksOptions; task, depth, children on TaskTreeItem; data, tree, loading, error, reload on UseTasksResult) and their types unchanged, ensuring no behavioral or API changes where these symbols are used elsewhere.packages/editor/src/extensions/task-view-embed/TaskViewEmbedExtension.ts (1)
11-25: Prefertypealiases overinterfacefor these exported object shapes.This keeps the file aligned with the repository TypeScript conventions.
As per coding guidelines, "Prefer
typeoverinterfacefor defining object shapes in TypeScript".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/editor/src/extensions/task-view-embed/TaskViewEmbedExtension.ts` around lines 11 - 25, Replace the exported interfaces with equivalent type aliases: change the exported TaskViewConfig interface to a type alias named TaskViewConfig and change TaskViewEmbedOptions interface to a type alias named TaskViewEmbedOptions (preserving the same property names and types such as scope, assignee, dueDate, status, showHierarchy and the renderView/HTMLAttributes shape), ensuring the renderView callback signature and React.ReactNode return type remain unchanged; update any local imports/uses if necessary to match the type alias exports.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/electron/src/renderer/components/PageView.tsx`:
- Line 128: The forEach callback using presence.forEach((user) =>
addSuggestion(user)) triggers lint/suspicious/useIterableCallbackReturn; change
it to a block-bodied callback so the return is explicit and side-effects are
clear—replace the concise arrow with a block-bodied arrow function that calls
addSuggestion(user) inside (e.g., presence.forEach((user) => {
addSuggestion(user); })); update the callsite in PageView where presence.forEach
is invoked to use this form.
In `@apps/web/src/components/MyTasksPanel.tsx`:
- Around line 25-29: The hardcoded limit in the Page fetch (useQuery(PageSchema,
{ limit: 200 })) causes pageTitles (computed in the useMemo that maps pages to
titles) to miss pages beyond 200; change the fetch to either remove the limit
or, better, fetch only the pages referenced by tasks by passing a filter with
the set of task.page IDs (e.g. build an array of unique task.page IDs and call
useQuery(PageSchema, { filter: { id: { in: [...] } } })), then recompute
pageTitles in the useMemo based on that result so all linked tasks get their
title chips.
In `@apps/web/src/components/PageTasksPanel.tsx`:
- Around line 18-20: isOverdue() currently compares raw milliseconds against
Date.now() which conflicts with other logic that normalizes due dates to UTC
midnight; update isOverdue (in PageTasksPanel.tsx) to perform a day-boundary
check like the rest of the codebase by comparing the task's UTC-day timestamp
against the start of today in UTC (use the same helper used elsewhere, e.g.
getStartOfUtcDay() or the toDateTimestamp() normalization) so overdue only
becomes true after the entire due day has passed.
In
`@docs/explorations/0103_`[_]_TASKS_EMBEDDED_IN_PAGES_BACKED_BY_NODES_MENTIONS_DUE_DATES_NESTED_SUBTASKS_DATABASES_CANVASES_AND_CROSS_SURFACE_TASK_MODEL.md:
- Line 220: The inline code span contains a trailing space (`[] `) which
triggers MD038; edit the markdown at the occurrence of the code span (the `[] `
token) and remove the trailing space so it becomes `` `[]` ``, ensuring the code
span has no internal trailing whitespace.
In `@packages/editor/src/extensions/task-view-embed/TaskViewEmbedExtension.ts`:
- Around line 35-37: parseTaskViewConfig currently returns the shared
DEFAULT_TASK_VIEW_CONFIG constant on fallback, which risks cross-node mutations;
change it to return a fresh copy instead (e.g., clone DEFAULT_TASK_VIEW_CONFIG
via structured cloning or by shallow copying the top-level object and
deep-copying nested arrays/objects) so callers get an independent object; update
every other fallback in this file that returns DEFAULT_TASK_VIEW_CONFIG to do
the same (search for other returns of DEFAULT_TASK_VIEW_CONFIG and replace with
a fresh-clone expression).
- Around line 141-152: The updateTaskViewEmbed action currently replaces the
entire viewConfig with DEFAULT_TASK_VIEW_CONFIG merged only with
options.viewConfig, which resets unspecified fields; fix it by first reading the
existing attributes for this.name (e.g., via
commands.editor?.getAttributes(this.name) or the current node attrs), then
construct viewConfig as { ...DEFAULT_TASK_VIEW_CONFIG, ...(existing.viewConfig
?? {}), ...(options.viewConfig ?? {}) } and pass that to
commands.updateAttributes(this.name, { ...(options.viewType !== undefined && {
viewType: options.viewType }), viewConfig }); this preserves previously set
fields while applying partial updates.
In `@packages/react/src/hooks/useTasks.ts`:
- Around line 114-143: buildTaskTree currently sets depth while linking nodes
which makes depth order-dependent; instead first build all TaskTreeItem nodes in
treeById and link parent/child relationships (as you already do), then compute
depths by traversing from root items: find roots (nodes with no parent) and run
a simple DFS/BFS that calls a helper like setDepth(node, depth) to assign
node.depth = depth and then recurse into node.children with depth+1; update
references to TaskNode/TaskTreeItem and the treeById/roots variables so depth
assignment happens only during this traversal.
---
Duplicate comments:
In `@apps/web/src/routes/doc`.$docId.tsx:
- Line 102: The one-line arrow callback used with presence.forEach is flagged by
the linter; change the callback to a block-bodied form so the forEach invocation
becomes a callback that calls addSuggestion(user) inside a block (e.g.,
presence.forEach((user) => { addSuggestion(user); })), ensuring the symbol
presence.forEach and the function addSuggestion are used unchanged but the
callback has a block body to satisfy Biome's lint rule.
In `@packages/editor/src/components/RichTextEditor.tsx`:
- Around line 515-516: The initial call to publishPageTasks() runs before
registering the editor.update listener, so updates emitted during
ensurePageTaskAttrs(editor) can be missed; fix by registering the listener first
(call editor.on('update', publishPageTasks) before calling publishPageTasks())
so that any update fired during ensurePageTaskAttrs(editor) will be handled by
the publishPageTasks handler.
---
Nitpick comments:
In `@apps/electron/src/renderer/components/PageTasksPanel.tsx`:
- Around line 5-7: Replace the interface PageTasksPanelProps with a type alias
(e.g., type PageTasksPanelProps = { pageId: string }) and update the exported
component declaration to include an explicit return type (e.g., export const
PageTasksPanel = (props: PageTasksPanelProps): JSX.Element => { ... }); ensure
you update any other occurrence noted (the similar definition at the other spot)
to follow the same pattern so props use a type alias and the component has an
explicit JSX.Element return type.
- Around line 22-115: The PageTasksPanel component is duplicated in web and
electron; extract a single shared component to avoid drift by moving the
implementation of PageTasksPanel (which uses useTasks, formatDueDate, isOverdue
and icons like CheckSquare2, Square, Calendar, Users) into a shared UI package
and have both apps import it; update the electron and web exports to import the
shared PageTasksPanel and ensure any app-specific dependencies (styling classes
or icon imports) are either re-exported from the shared package or injected via
props (e.g., pass icons or className overrides) so the shared component compiles
in both environments.
In `@apps/electron/src/renderer/components/PageView.tsx`:
- Around line 107-131: Extract the duplicate mention-suggestion logic into a
shared helper (e.g., buildMentionSuggestions) and replace the inline useMemo in
PageView with a call to that helper: move the Map creation, addSuggestion
function, and final Array.from(suggestions.values()) into the new utility which
accepts (presence, did, optional isLocal label/subtitle override) and returns
the suggestions array; update PageView to import and call
buildMentionSuggestions(presence, did) (and likewise update
apps/web/src/routes/doc.$docId.tsx to use the same helper) so both locations
reuse the same mentionSuggestions construction logic (referencing the existing
symbols mentionSuggestions, addSuggestion, presence, did, and the useMemo
wrapper).
In `@apps/web/src/components/Editor.tsx`:
- Around line 42-53: The exported Editor function lacks an explicit return type;
update its signature (the export function Editor(...) declaration) to include a
JSX return type (e.g., ): JSX.Element or React.JSX.Element) so the exported
component has an explicit return type per project rules; adjust any imports if
your chosen annotation requires them and ensure the Props type remains
unchanged.
In `@apps/web/src/components/MyTasksPanel.tsx`:
- Around line 1-5: Reorder the import block so all external packages (e.g.,
'@tanstack/react-router', 'lucide-react', 'react' / 'useMemo, useState') appear
before internal `@xnetjs/`* imports (move "PageSchema", "useIdentity", "useQuery",
"useTasks" after external imports), and add an explicit return type to the
exported component signature for MyTasksPanel (e.g., export function
MyTasksPanel(...): JSX.Element { ... }) so the function has a clear typed
return; apply the same import ordering and explicit return type adjustment to
the other occurrence noted.
In `@apps/web/src/components/PageTasksPanel.tsx`:
- Around line 5-7: Replace the PageTasksPanelProps interface with a type alias
to follow the project convention: change the declaration named
PageTasksPanelProps from an `interface` to a `type` (e.g., `type
PageTasksPanelProps = { pageId: string }`) and ensure any usages inside the
PageTasksPanel component still refer to PageTasksPanelProps without other
changes.
In `@packages/editor/src/components/RichTextEditor.tsx`:
- Around line 4-14: Reorder the type imports in RichTextEditor.tsx so external
type-only imports come first and the local relative import comes last: move the
imports from '@tiptap/core' (AnyExtension), 'y-protocols/awareness' (Awareness),
and 'yjs' (Y) to appear before the local '../extensions' type import
(DatabaseViewType, PageTaskSnapshot, SlashCommandItem, TaskMentionSuggestion,
TaskViewConfig, TaskViewEmbedType) so the module order follows the repository
rules (external packages, then internal packages, then local relative).
In `@packages/editor/src/extensions/task-view-embed/index.ts`:
- Line 1: This file is missing the required top-of-file JSDoc banner; add a
file-level JSDoc comment at the very top of the file (above the existing export
line) describing the module and any relevant metadata per project guidelines so
that the file exporting TaskViewEmbedExtension and DEFAULT_TASK_VIEW_CONFIG
includes the mandated header; ensure the comment follows the same JSDoc format
used across packages (brief description, optional `@module/`@file or
author/license tags if used in other files).
In `@packages/editor/src/extensions/task-view-embed/TaskViewEmbedExtension.ts`:
- Around line 11-25: Replace the exported interfaces with equivalent type
aliases: change the exported TaskViewConfig interface to a type alias named
TaskViewConfig and change TaskViewEmbedOptions interface to a type alias named
TaskViewEmbedOptions (preserving the same property names and types such as
scope, assignee, dueDate, status, showHierarchy and the
renderView/HTMLAttributes shape), ensuring the renderView callback signature and
React.ReactNode return type remain unchanged; update any local imports/uses if
necessary to match the type alias exports.
In `@packages/editor/src/extensions/task-view-embed/TaskViewEmbedNodeView.tsx`:
- Around line 1-190: Add the required file-level JSDoc and section dividers and
add JSDoc for exported items: insert a top-of-file JSDoc block describing the
package/editor extension file, its purpose and exports; add clear section
dividers (e.g., “// Constants”, “// Helpers”, “// Components / NodeViews”)
around SCOPE_OPTIONS/ASSIGNEE_OPTIONS/STATUS_OPTIONS/DUE_DATE_OPTIONS, getConfig
and FilterSelect, and the main exported function; and add a JSDoc comment for
the exported TaskViewEmbedNodeView function that documents parameters and
includes an `@example` showing basic usage (call signature showing node props or
usage with extension.options.renderView). Ensure the exported symbol name
TaskViewEmbedNodeView and helper names getConfig and FilterSelect are referenced
in the comments where appropriate.
In `@packages/react/src/components/TaskCollectionEmbed.tsx`:
- Around line 1-12: Replace the exported interface TaskCollectionEmbedProps with
a type alias (e.g., "type TaskCollectionEmbedProps = { ... }") preserving the
same property names and types, and add the required file-level JSDoc header at
the very top of the TaskCollectionEmbed.tsx file (above imports) following
package conventions; update any references to TaskCollectionEmbedProps if needed
to ensure exports/imports remain correct.
In `@packages/react/src/hooks/useTasks.ts`:
- Around line 12-33: Replace the exported interfaces UseTasksOptions,
TaskTreeItem, and UseTasksResult with equivalent exported type aliases to match
repo standards; keep the exact exported names and all properties (pageId,
assigneeDid, includeCompleted, statuses, parentTaskId, dueDateFilter on
UseTasksOptions; task, depth, children on TaskTreeItem; data, tree, loading,
error, reload on UseTasksResult) and their types unchanged, ensuring no
behavioral or API changes where these symbols are used elsewhere.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9e85203e-97d7-43cb-a1ce-0f2118dbfa9b
📒 Files selected for processing (22)
apps/electron/src/renderer/components/PageTasksPanel.tsxapps/electron/src/renderer/components/PageView.tsxapps/web/src/components/Editor.tsxapps/web/src/components/MyTasksPanel.tsxapps/web/src/components/PageTasksPanel.tsxapps/web/src/components/Sidebar.tsxapps/web/src/routes/doc.$docId.tsxdocs/explorations/0103_[_]_TASKS_EMBEDDED_IN_PAGES_BACKED_BY_NODES_MENTIONS_DUE_DATES_NESTED_SUBTASKS_DATABASES_CANVASES_AND_CROSS_SURFACE_TASK_MODEL.mdpackages/editor/src/components/RichTextEditor.tsxpackages/editor/src/extensions.tspackages/editor/src/extensions/slash-command/items.tspackages/editor/src/extensions/task-view-embed/TaskViewEmbedExtension.test.tspackages/editor/src/extensions/task-view-embed/TaskViewEmbedExtension.tspackages/editor/src/extensions/task-view-embed/TaskViewEmbedNodeView.tsxpackages/editor/src/extensions/task-view-embed/index.tspackages/editor/src/react.tspackages/react/src/components/TaskCollectionEmbed.tsxpackages/react/src/hooks/usePageTaskSync.test.tsxpackages/react/src/hooks/usePageTaskSync.tspackages/react/src/hooks/useTasks.test.tsxpackages/react/src/hooks/useTasks.tspackages/react/src/index.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/react/src/hooks/usePageTaskSync.test.tsx
- packages/react/src/hooks/usePageTaskSync.ts
- packages/react/src/index.ts
| function isOverdue(timestamp: number | undefined, completed: boolean): boolean { | ||
| return typeof timestamp === 'number' && !completed && timestamp < Date.now() | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find and inspect the files mentioned in the review
# 1) Check usePageTaskSync.ts for dueDate timestamp conversion
echo "=== Checking usePageTaskSync.ts ==="
find . -type f -name "usePageTaskSync.ts" | head -5
# 2) Check useTasks.ts for filter logic
echo "=== Checking useTasks.ts ==="
find . -type f -name "useTasks.ts" | head -5
# 3) Also look for PageTasksPanel.tsx to verify the context
echo "=== Checking PageTasksPanel.tsx ==="
find . -type f -name "PageTasksPanel.tsx" | head -5Repository: crs48/xNet
Length of output: 343
🏁 Script executed:
#!/bin/bash
# Search for dueDate and timestamp related code
rg -n 'dueDate|toDateTimestamp|matchesDueDate' --type ts --type tsx | head -50Repository: crs48/xNet
Length of output: 81
🏁 Script executed:
#!/bin/bash
# Search for dueDate and timestamp handling without file type restriction
echo "=== Searching for dueDate and timestamp patterns ==="
rg -n 'dueDate|toDateTimestamp|matchesDueDate' -A 2 -B 2 | head -80
# Also check the PageTasksPanel.tsx file directly
echo "=== Checking PageTasksPanel.tsx ==="
cat -n ./apps/web/src/components/PageTasksPanel.tsx | head -40Repository: crs48/xNet
Length of output: 6822
🏁 Script executed:
#!/bin/bash
# Check usePageTaskSync.ts for how dueDate is processed
echo "=== usePageTaskSync.ts content ==="
wc -l ./packages/react/src/hooks/usePageTaskSync.ts
head -100 ./packages/react/src/hooks/usePageTaskSync.tsRepository: crs48/xNet
Length of output: 3058
🏁 Script executed:
#!/bin/bash
# Check useTasks.ts for filter/comparison logic
echo "=== useTasks.ts content ==="
wc -l ./packages/react/src/hooks/useTasks.ts
cat -n ./packages/react/src/hooks/useTasks.ts | head -100Repository: crs48/xNet
Length of output: 4340
🏁 Script executed:
#!/bin/bash
# Get the complete matchesDueDate function from useTasks.ts
sed -n '92,130p' ./packages/react/src/hooks/useTasks.tsRepository: crs48/xNet
Length of output: 1163
🏁 Script executed:
#!/bin/bash
# Check if there's a date conversion happening - search for where dueDate string becomes number
rg -n 'dueDate.*timestamp|toDate|parseDate|new Date.*dueDate' ./packages/react/src/hooks/ -A 2Repository: crs48/xNet
Length of output: 663
🏁 Script executed:
#!/bin/bash
# Get the complete toDateTimestamp function
sed -n '106,120p' ./packages/react/src/hooks/usePageTaskSync.tsRepository: crs48/xNet
Length of output: 552
🏁 Script executed:
#!/bin/bash
# Also check if there's any documentation or usage that clarifies the timezone handling
rg -n 'getStartOfUtcDay|toDateTimestamp|dueDate.*UTC' ./packages/react/src/hooks/ -B 2 -A 3Repository: crs48/xNet
Length of output: 3143
isOverdue() uses raw millisecond comparison but timestamps are normalized to UTC day boundaries.
Line 19 compares timestamp < Date.now() while toDateTimestamp() (usePageTaskSync.ts) stores due dates as UTC midnight via Date.UTC(year, month - 1, day). Meanwhile, useTasks.ts implements matchesDueDate('overdue') using day-boundary comparison with getStartOfUtcDay(). This causes inconsistent overdue semantics: PageTasksPanel marks tasks overdue immediately after midnight UTC, while useTasks marks them overdue only after the entire due day passes. Align isOverdue() to use day-level semantics matching the rest of the task filtering logic.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/components/PageTasksPanel.tsx` around lines 18 - 20, isOverdue()
currently compares raw milliseconds against Date.now() which conflicts with
other logic that normalizes due dates to UTC midnight; update isOverdue (in
PageTasksPanel.tsx) to perform a day-boundary check like the rest of the
codebase by comparing the task's UTC-day timestamp against the start of today in
UTC (use the same helper used elsewhere, e.g. getStartOfUtcDay() or the
toDateTimestamp() normalization) so overdue only becomes true after the entire
due day has passed.
| const pageId = scope === 'current-page' ? currentPageId : undefined | ||
| const assigneeDid = assignee === 'me' ? currentDid : undefined | ||
| const statuses = status === 'done' ? (['done'] as const) : undefined | ||
| const includeCompleted = status !== 'open' | ||
| const { data, tree, loading } = useTasks({ | ||
| pageId, | ||
| assigneeDid, | ||
| includeCompleted, | ||
| statuses: statuses ? [...statuses] : undefined, | ||
| dueDateFilter: dueDate | ||
| }) |
There was a problem hiding this comment.
Missing page context still triggers a global task query.
When scope is 'current-page' and currentPageId is null, Line 53 passes a falsy pageId, so useTasks falls back to an unscoped query before Line 77 returns the context warning.
🔧 Suggested fix
export function TaskCollectionEmbed({
@@
}: TaskCollectionEmbedProps): JSX.Element {
- const pageId = scope === 'current-page' ? currentPageId : undefined
+ const missingPageContext = scope === 'current-page' && !currentPageId
+ const pageId =
+ scope === 'current-page'
+ ? (currentPageId ?? '__task_collection_embed_missing_page_context__')
+ : undefined
@@
const { data, tree, loading } = useTasks({
@@
})
- if (scope === 'current-page' && !currentPageId) {
+ if (missingPageContext) {
return <div className="p-4 text-sm text-muted-foreground">This view needs a page context.</div>
}Also applies to: 77-79
- replace single-run edge bundling timing with a median of repeated samples - scale canvas and view performance budgets for local versus CI variance - keep the assertions as regression guards without depending on micro-benchmark precision
There was a problem hiding this comment.
♻️ Duplicate comments (2)
apps/electron/src/renderer/components/PageView.tsx (1)
130-130:⚠️ Potential issue | 🟡 MinorBiome lint issue is still present on the
presence.forEachcallback.Switch to a block-bodied callback so
forEachdoes not implicitly return a value.🔧 Suggested fix
- presence.forEach((user) => addSuggestion(user)) + presence.forEach((user) => { + addSuggestion(user) + })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/electron/src/renderer/components/PageView.tsx` at line 130, The forEach callback currently uses a concise arrow body (presence.forEach((user) => addSuggestion(user))) which causes an implicit return and triggers the Biome lint rule; change it to a block-bodied callback so it does not return a value (e.g., presence.forEach((user) => { addSuggestion(user); })) — update the occurrence in the PageView component where presence.forEach is called and ensure addSuggestion is invoked inside a block-bodied arrow function.apps/web/src/routes/doc.$docId.tsx (1)
103-103:⚠️ Potential issue | 🟡 MinorBiome lint issue is still present on the
presence.forEachcallback.Use a block-bodied callback to avoid returning a value from
forEach.🔧 Suggested fix
- presence.forEach((user) => addSuggestion(user)) + presence.forEach((user) => { + addSuggestion(user) + })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/routes/doc`.$docId.tsx at line 103, The forEach callback for presence.forEach currently uses a concise arrow body which can implicitly return a value; change it to a block-bodied callback so nothing is returned—replace presence.forEach((user) => addSuggestion(user)) with a block-bodied form that calls addSuggestion(user) inside braces (e.g., presence.forEach((user) => { addSuggestion(user); })), ensuring the callback has no return value and referencing the presence.forEach and addSuggestion symbols.
🧹 Nitpick comments (3)
packages/editor/src/utils/gravatar.ts (1)
1-94: Document this new package utility module per repo standards.Please add a file-level JSDoc header and JSDoc blocks (with
@example) for exported helpers (createGravatarUrl,md5) so this module aligns with package conventions.As per coding guidelines:
packages/**/*.{ts,tsx}: Include file-level JSDoc at top of fileandFunction documentation should include JSDoc with@exampletags.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/editor/src/utils/gravatar.ts` around lines 1 - 94, Add a file-level JSDoc header at the top of the file and JSDoc blocks for the exported functions createGravatarUrl and md5 (including `@param`, `@returns` and an `@example` for each) so the module follows package documentation conventions; update the JSDoc to describe the module purpose (Gravatar utility), document createGravatarUrl(input: string, size?: number) and md5(input: string) with concise descriptions and usage examples, and ensure exported symbols (createGravatarUrl, md5) are referenced in their respective comments.packages/editor/src/components/TaskMentionMenu.test.tsx (1)
1-1: Add the package-level JSDoc header to this new test module.This file currently starts with imports; package files are expected to begin with the standard file-level JSDoc banner.
🔧 Suggested change
+/** + * `@xnetjs/editor` - Task mention menu tests + */ import { render, screen } from '@testing-library/react'As per coding guidelines:
packages/**/*.{ts,tsx}: Include file-level JSDoc at top of file: /**@xnetjs/sync- Unified sync primitives for xNet */.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/editor/src/components/TaskMentionMenu.test.tsx` at line 1, This test module (TaskMentionMenu.test.tsx) is missing the required file-level JSDoc banner; add the standard package-level JSDoc comment as the very first lines of the file (before any imports) using the exact header string: /** `@xnetjs/sync` - Unified sync primitives for xNet */ so the file begins with that banner then the existing imports (e.g., render, screen from '@testing-library/react').apps/electron/src/renderer/components/PageView.tsx (1)
780-810: Add an Electron Playwright path for this new integrated editor flow.This segment now wires mentions, page-task sync, task-view rendering, and panel UI together; a focused renderer e2e would reduce regression risk.
Based on learnings:
Test UI changes in Electron with Playwright after implementing.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/electron/src/renderer/components/PageView.tsx` around lines 780 - 810, Add an Electron Playwright E2E path that exercises the new integrated editor flow wired here: render the editor with mentionSuggestions, trigger onPageTasksChange (handleTasksChange) and verify renderTaskView via TaskCollectionEmbed props (scope/assignee/dueDate/status/showHierarchy), ensure OrphanedThreadList appears and can toggle/dismiss/reattach (onToggleCollapse/onDismiss/onReattach/onSelect) and that PageTasksPanel (pageId prop) is visible and syncs with task changes; create a new Playwright test under the Electron e2e suite that launches the app, navigates to the page with docId/did test fixtures, performs UI actions to invoke mentions and task edits, and asserts the expected DOM/state updates to prevent regressions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@apps/electron/src/renderer/components/PageView.tsx`:
- Line 130: The forEach callback currently uses a concise arrow body
(presence.forEach((user) => addSuggestion(user))) which causes an implicit
return and triggers the Biome lint rule; change it to a block-bodied callback so
it does not return a value (e.g., presence.forEach((user) => {
addSuggestion(user); })) — update the occurrence in the PageView component where
presence.forEach is called and ensure addSuggestion is invoked inside a
block-bodied arrow function.
In `@apps/web/src/routes/doc`.$docId.tsx:
- Line 103: The forEach callback for presence.forEach currently uses a concise
arrow body which can implicitly return a value; change it to a block-bodied
callback so nothing is returned—replace presence.forEach((user) =>
addSuggestion(user)) with a block-bodied form that calls addSuggestion(user)
inside braces (e.g., presence.forEach((user) => { addSuggestion(user); })),
ensuring the callback has no return value and referencing the presence.forEach
and addSuggestion symbols.
---
Nitpick comments:
In `@apps/electron/src/renderer/components/PageView.tsx`:
- Around line 780-810: Add an Electron Playwright E2E path that exercises the
new integrated editor flow wired here: render the editor with
mentionSuggestions, trigger onPageTasksChange (handleTasksChange) and verify
renderTaskView via TaskCollectionEmbed props
(scope/assignee/dueDate/status/showHierarchy), ensure OrphanedThreadList appears
and can toggle/dismiss/reattach (onToggleCollapse/onDismiss/onReattach/onSelect)
and that PageTasksPanel (pageId prop) is visible and syncs with task changes;
create a new Playwright test under the Electron e2e suite that launches the app,
navigates to the page with docId/did test fixtures, performs UI actions to
invoke mentions and task edits, and asserts the expected DOM/state updates to
prevent regressions.
In `@packages/editor/src/components/TaskMentionMenu.test.tsx`:
- Line 1: This test module (TaskMentionMenu.test.tsx) is missing the required
file-level JSDoc banner; add the standard package-level JSDoc comment as the
very first lines of the file (before any imports) using the exact header string:
/** `@xnetjs/sync` - Unified sync primitives for xNet */ so the file begins with
that banner then the existing imports (e.g., render, screen from
'@testing-library/react').
In `@packages/editor/src/utils/gravatar.ts`:
- Around line 1-94: Add a file-level JSDoc header at the top of the file and
JSDoc blocks for the exported functions createGravatarUrl and md5 (including
`@param`, `@returns` and an `@example` for each) so the module follows package
documentation conventions; update the JSDoc to describe the module purpose
(Gravatar utility), document createGravatarUrl(input: string, size?: number) and
md5(input: string) with concise descriptions and usage examples, and ensure
exported symbols (createGravatarUrl, md5) are referenced in their respective
comments.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 822b1213-ac75-4ff6-aae1-ad733b254d86
📒 Files selected for processing (8)
apps/electron/src/renderer/components/PageView.tsxapps/web/src/routes/doc.$docId.tsxpackages/editor/src/components/TaskMentionMenu.test.tsxpackages/editor/src/components/TaskMentionMenu.tsxpackages/editor/src/extensions.tspackages/editor/src/react.tspackages/editor/src/utils/gravatar.test.tspackages/editor/src/utils/gravatar.ts
✅ Files skipped from review due to trivial changes (1)
- packages/editor/src/utils/gravatar.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/editor/src/components/TaskMentionMenu.tsx
- packages/editor/src/extensions.ts
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/canvas/src/__tests__/edge-bundling.test.ts (1)
329-332: Consider separating the correctness assertion from the timed callback.The
expect(bundles.length).toBeGreaterThan(0)runs multiple times (once per sample plus warm-up). While the overhead is minimal and the approach works, separating the performance measurement from the correctness assertion would make the test intent clearer.♻️ Optional refactor to separate concerns
- const elapsed = getMedianDuration(() => { - const bundles = b.bundle(edges, positions) - expect(bundles.length).toBeGreaterThan(0) - }) + let bundles: ReturnType<typeof b.bundle> = [] + const elapsed = getMedianDuration(() => { + bundles = b.bundle(edges, positions) + }) + + expect(bundles.length).toBeGreaterThan(0)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/canvas/src/__tests__/edge-bundling.test.ts` around lines 329 - 332, The test is asserting correctness inside the timed callback which causes the expectation to run multiple times; change it so getMedianDuration(() => b.bundle(edges, positions)) only measures performance and does not include the expect, then call b.bundle(edges, positions) once (or reuse the last result) after timing and assert expect(bundles.length).toBeGreaterThan(0). Update references to getMedianDuration, b.bundle and bundles accordingly so the timing loop and correctness assertion are separate.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/canvas/src/__tests__/minimap.test.ts`:
- Around line 11-13: The helper getPerformanceBudget lacks JSDoc with an
`@example` and uses a truthy check on process.env.CI; update getPerformanceBudget
to add JSDoc describing parameters/return and include an `@example` showing
typical usage, and change the CI check from process.env.CI to an explicit
comparison matching vitest.config.ts (e.g., process.env.CI === 'true') so values
like "false" aren't treated as truthy; apply the same explicit CI check pattern
to the other mentioned test helpers for consistency.
In `@packages/views/src/__tests__/virtualized-table.test.tsx`:
- Around line 20-22: Add a JSDoc block to the getPerformanceBudget function that
documents parameters, return value, and includes an `@example` showing typical
usage (e.g., passing local and CI budgets and the expected returned value
depending on process.env.CI). Ensure the comment sits immediately above the
getPerformanceBudget declaration and uses standard JSDoc tags like `@param` and
`@returns` plus an `@example` snippet demonstrating both CI and local outcomes.
---
Nitpick comments:
In `@packages/canvas/src/__tests__/edge-bundling.test.ts`:
- Around line 329-332: The test is asserting correctness inside the timed
callback which causes the expectation to run multiple times; change it so
getMedianDuration(() => b.bundle(edges, positions)) only measures performance
and does not include the expect, then call b.bundle(edges, positions) once (or
reuse the last result) after timing and assert
expect(bundles.length).toBeGreaterThan(0). Update references to
getMedianDuration, b.bundle and bundles accordingly so the timing loop and
correctness assertion are separate.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 20473016-6a9d-4a80-8467-35cf98153265
📒 Files selected for processing (4)
packages/canvas/src/__tests__/edge-bundling.test.tspackages/canvas/src/__tests__/minimap.test.tspackages/views/src/__tests__/editor-performance.test.tsxpackages/views/src/__tests__/virtualized-table.test.tsx
| function getPerformanceBudget(localBudgetMs: number, ciBudgetMs: number): number { | ||
| return process.env.CI ? ciBudgetMs : localBudgetMs | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check if the file exists and inspect the relevant lines
head -n 20 packages/canvas/src/__tests__/minimap.test.tsRepository: crs48/xNet
Length of output: 558
🏁 Script executed:
# Run the verification script from the review to check CI patterns
fd -t f -e yml -e yaml -e json -e ts -e tsx -e js -e jsx -e sh -0 \
| xargs -0 rg -n --no-heading -e '(^|[[:space:]])CI[[:space:]]*[:=][[:space:]]*["'"'"'](false|0|no)["'"'"']?' -e 'process\.env\.CI' | head -50Repository: crs48/xNet
Length of output: 659
🏁 Script executed:
# Check the full structure of the test file to understand context
wc -l packages/canvas/src/__tests__/minimap.test.tsRepository: crs48/xNet
Length of output: 103
Add JSDoc with @example to the performance budget helper.
This function must include JSDoc documentation per the packages/**/*.{ts,tsx} guideline. Additionally, process.env.CI ? ... treats values like "false" as truthy; use explicit comparison matching the pattern already established in vitest.config.ts for safer CI detection.
Proposed fix
+/**
+ * Returns environment-aware performance budget for timing assertions.
+ *
+ * `@example`
+ * const budget = getPerformanceBudget(100, 200)
+ */
function getPerformanceBudget(localBudgetMs: number, ciBudgetMs: number): number {
- return process.env.CI ? ciBudgetMs : localBudgetMs
+ const isCi = process.env.CI === 'true'
+ return isCi ? ciBudgetMs : localBudgetMs
}Note: The same pattern exists in packages/views/src/__tests__/virtualized-table.test.tsx:21, packages/views/src/__tests__/editor-performance.test.tsx:7, and packages/canvas/src/__tests__/edge-bundling.test.ts:17 and should be updated for consistency with vitest.config.ts:5.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function getPerformanceBudget(localBudgetMs: number, ciBudgetMs: number): number { | |
| return process.env.CI ? ciBudgetMs : localBudgetMs | |
| } | |
| /** | |
| * Returns environment-aware performance budget for timing assertions. | |
| * | |
| * `@example` | |
| * const budget = getPerformanceBudget(100, 200) | |
| */ | |
| function getPerformanceBudget(localBudgetMs: number, ciBudgetMs: number): number { | |
| const isCi = process.env.CI === 'true' | |
| return isCi ? ciBudgetMs : localBudgetMs | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/canvas/src/__tests__/minimap.test.ts` around lines 11 - 13, The
helper getPerformanceBudget lacks JSDoc with an `@example` and uses a truthy check
on process.env.CI; update getPerformanceBudget to add JSDoc describing
parameters/return and include an `@example` showing typical usage, and change the
CI check from process.env.CI to an explicit comparison matching vitest.config.ts
(e.g., process.env.CI === 'true') so values like "false" aren't treated as
truthy; apply the same explicit CI check pattern to the other mentioned test
helpers for consistency.
| function getPerformanceBudget(localBudgetMs: number, ciBudgetMs: number): number { | ||
| return process.env.CI ? ciBudgetMs : localBudgetMs | ||
| } |
There was a problem hiding this comment.
Add JSDoc (with @example) for the new helper.
getPerformanceBudget is a new function in a packages/**/*.{ts,tsx} file and should include function-level JSDoc with an example.
Proposed update
+/**
+ * Returns a test performance budget adjusted for local vs CI environments.
+ *
+ * `@example`
+ * const budget = getPerformanceBudget(700, 1200)
+ */
function getPerformanceBudget(localBudgetMs: number, ciBudgetMs: number): number {
return process.env.CI ? ciBudgetMs : localBudgetMs
}As per coding guidelines: packages/**/*.{ts,tsx}: Function documentation should include JSDoc with @example tags.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function getPerformanceBudget(localBudgetMs: number, ciBudgetMs: number): number { | |
| return process.env.CI ? ciBudgetMs : localBudgetMs | |
| } | |
| /** | |
| * Returns a test performance budget adjusted for local vs CI environments. | |
| * | |
| * `@example` | |
| * const budget = getPerformanceBudget(700, 1200) | |
| */ | |
| function getPerformanceBudget(localBudgetMs: number, ciBudgetMs: number): number { | |
| return process.env.CI ? ciBudgetMs : localBudgetMs | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/views/src/__tests__/virtualized-table.test.tsx` around lines 20 -
22, Add a JSDoc block to the getPerformanceBudget function that documents
parameters, return value, and includes an `@example` showing typical usage (e.g.,
passing local and CI budgets and the expected returned value depending on
process.env.CI). Ensure the comment sits immediately above the
getPerformanceBudget declaration and uses standard JSDoc tags like `@param` and
`@returns` plus an `@example` snippet demonstrating both CI and local outcomes.
- share page task panels and mention suggestion construction across web and desktop - harden task sync, due date, and task view updates against stale or partial state - align overdue logic and task tree depth handling with canonical task semantics
| function getTodayDateString(): string { | ||
| const now = new Date() | ||
| const year = now.getFullYear() | ||
| const month = String(now.getMonth() + 1).padStart(2, '0') | ||
| const day = String(now.getDate()).padStart(2, '0') | ||
| return `${year}-${month}-${day}` | ||
| } |
There was a problem hiding this comment.
Overdue indicator uses local time; data layer uses UTC
getTodayDateString() (lines 73–78) derives today's date from new Date() using local getters (getFullYear, getMonth, getDate), so the "today" string varies by the viewer's timezone. By contrast, useTasks.ts and TaskCollectionEmbed.tsx both call getStartOfUtcDay(Date.now()) for overdue comparisons.
This means the data-overdue attribute on the inline chip and the overdue indicator in the embed panel can disagree for dates near midnight UTC. A task due today in UTC may render as overdue in the chip for users in UTC+X timezones, while the embed panel still shows it as on-time.
Consider normalising to UTC in getTodayDateString:
| function getTodayDateString(): string { | |
| const now = new Date() | |
| const year = now.getFullYear() | |
| const month = String(now.getMonth() + 1).padStart(2, '0') | |
| const day = String(now.getDate()).padStart(2, '0') | |
| return `${year}-${month}-${day}` | |
| } | |
| function getTodayDateString(): string { | |
| const now = new Date() | |
| const year = now.getUTCFullYear() | |
| const month = String(now.getUTCMonth() + 1).padStart(2, '0') | |
| const day = String(now.getUTCDate()).padStart(2, '0') | |
| return `${year}-${month}-${day}` | |
| } |
| const handlePickDueDate = useCallback(async () => { | ||
| const selectedDate = await pickDate(getCurrentTaskDueDate(editor)) | ||
| if (!selectedDate) return | ||
|
|
||
| editor.chain().focus().setTaskDueDate(selectedDate).run() | ||
| }, [editor]) |
There was a problem hiding this comment.
No path to clear an existing due date via the toolbar
handlePickDueDate only calls setTaskDueDate when selectedDate is non-null; dismissing the native picker (without picking a date) does nothing. There is no affordance to remove a due date once set.
TaskDueDateExtension already exposes clearTaskDueDate, so the button could toggle: if a due date is already set, clicking the button should clear it instead of opening the picker.
| const handlePickDueDate = useCallback(async () => { | |
| const selectedDate = await pickDate(getCurrentTaskDueDate(editor)) | |
| if (!selectedDate) return | |
| editor.chain().focus().setTaskDueDate(selectedDate).run() | |
| }, [editor]) | |
| const handlePickDueDate = useCallback(async () => { | |
| if (getCurrentTaskDueDate(editor) !== null) { | |
| editor.chain().focus().clearTaskDueDate().run() | |
| return | |
| } | |
| const selectedDate = await pickDate(null) | |
| if (!selectedDate) return | |
| editor.chain().focus().setTaskDueDate(selectedDate).run() | |
| }, [editor]) |
| function getStartOfUtcDay(timestamp: number): number { | ||
| const date = new Date(timestamp) | ||
| return Date.UTC(date.getUTCFullYear(), date.getUTCMonth(), date.getUTCDate()) | ||
| } |
There was a problem hiding this comment.
Duplicated getStartOfUtcDay helper
An identical getStartOfUtcDay function is defined in both packages/react/src/components/TaskCollectionEmbed.tsx (line 34) and packages/react/src/hooks/useTasks.ts (line 87). Since both files live in the same @xnetjs/react package, extracting this to a small shared utility (e.g., src/utils/dateUtils.ts) would eliminate the duplication and keep the overdue logic in one place.
- merge origin/main into codex/tasks-page-task-sync - preserve task-item toolbar visibility with derived selection state - prevent TaskCollectionEmbed from falling back to an unscoped query without page context - add regression coverage for missing page context handling - carry forward upstream CI and editor e2e updates
|
Preview: https://xnet.fyi/pr/4/app/ |
| function formatDueDate(timestamp: number | undefined): string | null { | ||
| if (typeof timestamp !== 'number') return null | ||
|
|
||
| return new Date(timestamp).toLocaleDateString(undefined, { | ||
| month: 'short', | ||
| day: 'numeric' | ||
| }) | ||
| } |
There was a problem hiding this comment.
formatDueDate shifts dates for users west of UTC
Timestamps are stored as UTC midnight (via Date.UTC(...) in usePageTaskSync). Converting with new Date(timestamp).toLocaleDateString() re-interprets that midnight instant in the viewer's local timezone. A user in UTC−8 sees midnight UTC as 4 PM the previous day, so a task due "Mar 20" is displayed as "Mar 19".
The inline chip in TaskDueDateExtension is rendered from the date string directly (new Date(year, month-1, day)) so it always shows the correct date. This creates a visible inconsistency: the panel label and the chip label disagree for the same task.
Fix by pinning the formatter to UTC:
| function formatDueDate(timestamp: number | undefined): string | null { | |
| if (typeof timestamp !== 'number') return null | |
| return new Date(timestamp).toLocaleDateString(undefined, { | |
| month: 'short', | |
| day: 'numeric' | |
| }) | |
| } | |
| function formatDueDate(timestamp: number | undefined): string | null { | |
| if (typeof timestamp !== 'number') return null | |
| return new Intl.DateTimeFormat(undefined, { | |
| month: 'short', | |
| day: 'numeric', | |
| timeZone: 'UTC' | |
| }).format(timestamp) | |
| } |
The same issue exists in packages/react/src/components/TaskCollectionEmbed.tsx at line 28.
| addSuggestion(did ? { did } : null, true) | ||
|
|
||
| for (const user of presence) { | ||
| addSuggestion(user) | ||
| } |
There was a problem hiding this comment.
Local user stub blocks their presence entry from providing a display name
addSuggestion({ did }, true) is called first, adding the current user with only a did field — no name, color, or avatar. The suggestions.has(entry.did) guard then prevents the richer presence entry (with name/color/avatar) from ever being used when the for...of presence loop runs.
As a result the local user's mention chip always shows a truncated DID (${did.slice(8, 16)}...) as its label, even when their full name is available in presence.
Fix: look up the presence entry for the local user first so that its name and avatar are used:
| addSuggestion(did ? { did } : null, true) | |
| for (const user of presence) { | |
| addSuggestion(user) | |
| } | |
| const presenceArray = [...presence] | |
| const localEntry = presenceArray.find((u) => u.did === did) | |
| // Add local user first (prefer presence data if available) | |
| addSuggestion(localEntry ?? (did ? { did } : null), true) | |
| for (const user of presenceArray) { | |
| addSuggestion(user) | |
| } |
feat(tasks): add inline assignees and due dates to page tasks
feat(tasks): add inline assignees and due dates to page tasks
Implements exploration **0244** — the fourth essay in the xNet blog
series (sea → soil → sky → **the wind**).
## What this is
A new blog post, *The Desert That Feeds the Forest*, that takes the
metaphors the user loved — **desert bees** and **the Sahara fertilizing
the Amazon** — and ties them to xNet, the open web, and Big Tech.
The spine: the important thing is almost never the thing on the surface.
A viral AI-clickbait video ("frozen bees in the Sahara") points,
accidentally, at two real and well-documented phenomena:
- The **dust bridge** — ~27.7M tons of Saharan dust a year carry ~22,000
tons of phosphorus to the Amazon, replacing almost exactly what the
rainforest loses to rain. The richest source is the Bodélé Depression:
fossil diatoms from a lake dead for millennia. *Death feeds life; the
wasteland is the supply line.*
- The **solitary desert bee** — an uncredited, invisible keystone
pollinator (not the branded honeybee).
…mapped onto the open web's invisible substrate (open protocols + the
maintainers Nadia Eghbal called the *"keystone species… invisible
because they work"*), with Big Tech as the monoculture plantation that
strip-mines the commons. xNet's line: **build like the dust, not the
plantation** — with Charter receipts (Own / Exit / Calm).
## Changes
- `site/src/pages/blog/the-desert-that-feeds-the-forest.astro` — the
essay
- `site/src/components/blog/DustHero.astro` — hero art (dust plume
arcing across an ocean; cosmic-X as a mid-Atlantic mote)
- `site/src/components/blog/DustBridge.astro` — the structural diagram
(Bodélé → Atlantic → Amazon → runoff, mapped to the web)
- `site/src/components/blog/HonestDesert.astro` — the self-audit panel
- `site/src/data/blog.ts` — metadata entry (reuses the `nature` tag;
index + RSS update automatically)
- changelog fragment + exploration doc checked off (`[_]`→`[x]`)
All art is inline SVG (no third-party assets — Self-Audit parity).
## Verification
- `pnpm --filter site build` ✅ (104 pages; the new page, index card, and
RSS entry all generated, newest-first)
- Rendered the built site and confirmed the hero, `DustBridge`, and
`HonestDesert` render with no console errors; OG title/description
populate.
- Prose honesty: the post never claims the "frozen bees" experiment was
real; figures are hedged ("roughly", "close to"); `HonestDesert`
discloses the simplifications.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
2 participants
Summary
@mentionassignee chips and due-date chips for page-backed task rowsTasknodes alongside existing task/page/reference reconciliationVerification
Notes
TasknodesSummary by CodeRabbit
New Features
UI Improvements
Documentation
Tests