Merge backfill feature branch#819
Closed
sij411 wants to merge 39 commits into
Closed
Conversation
Define the initial @fedify/backfill async generator API around a typed BackfillContext, note seed object, traversal options, and BackfillItem wrappers. The generator remains a stub so tests and traversal logic can be added in follow-up commits. Assisted-by: Codex:gpt-5
Add the initial context-posts traversal for @fedify/backfill. The implementation dereferences the seed object's context, accepts direct ActivityStreams collections and collection pages, yields post-like objects, and enforces request, item, interval, abort, and duplicate-id handling. Add tests for the PR 1 behavior across Deno, Node.js, and Bun. Assisted-by: Codex:gpt-5
Replace the scaffold status text with a short description of the initial context collection backfill behavior and a minimal usage example. Assisted-by: Codex:gpt-5
Remove unrelated lockfile churn from the backfill branch, keeping only the new package importer required for @fedify/backfill. Assisted-by: Codex:gpt-5
Skip collection URL items that fail to dereference instead of terminating the whole traversal, while still stopping when the request budget is exhausted. Also clean up interval abort listeners after successful waits. Assisted-by: Codex:gpt-5
Normalize every platform path separator in globbed test entries before passing those paths to tsdown. Assisted-by: Codex:gpt-5
When callers pass a string interval, report a clear error if Temporal is not available globally instead of failing with an unhelpful ReferenceError. Assisted-by: Codex:gpt-5
Make the interval option explicitly accept string durations, keep the internal duration helper aligned with that public type, and use strict Node assertions in the backfill tests. Explicitly externalize @fedify/vocab in the backfill tsdown build so vocabulary class identity remains stable for instanceof checks. Assisted-by: Codex:gpt-5
Replace the package README's VitePress-only installation block with plain Markdown, document the new public API surface with since tags, and mark exported option and result properties as readonly. Export the request-budget error class so callers can identify it when it escapes traversal internals. Assisted-by: Codex:gpt-5
Keep the new backfill API documentation on provisional 2.x.0 since tags until the feature branch is ready to merge and the target release version is known. Assisted-by: Codex:gpt-5
Add initial backfill package
Add context-auto, context-objects, and context-activities strategy handling for context collection backfill. The auto strategy classifies ordinary objects directly and extracts objects from supported Create activities while preserving shared request budgets, deduplication, abort, and interval behavior. Document the strategy behavior and add coverage for explicit, automatic, empty, overlapping, and duplicate strategy configurations. Assisted-by: Codex:gpt-5.5
Document the behavior of each context backfill strategy and make the context-auto precedence explicit in the public options comments. Tighten the README wording so it describes the currently supported Create activity handling instead of implying support for every Activity type. Assisted-by: Codex:gpt-5
Support FEP-f228 activity collection backfill
Extend the backfill public strategy and origin types for the upcoming reply-tree traversal. Clarify that context-auto absorbs only other context collection strategies so it can later compose with reply-tree traversal. Assisted-by: Codex:gpt-5.5
Run normalized strategies through a shared orchestration path so context collection traversal can compose with the upcoming reply-tree strategy. Keep context-auto absorbing only overlapping context collection strategies, while preserving global deduplication, budgets, and yield metadata handling. Assisted-by: Codex:gpt-5.5
Implement the reply-tree ancestor path through inReplyTo targets. Ancestors share the existing document loader, request budget, abort signal, and global output deduplication while keeping a separate visited set to prevent traversal cycles. Add coverage for embedded and dereferenced ancestors, maxDepth, maxRequests, cycle prevention, and deduplication against context collection results. Assisted-by: Codex:gpt-5.5
Extend reply-tree traversal to follow replies collections after ancestor lookup. Descendants reuse the existing collection loader, request budget, abort signal, and visited-state handling while reporting replies origin and reply-tree depth metadata. Add coverage for embedded and dereferenced replies collections, maxDepth, maxRequests, and descendant cycle prevention. Assisted-by: Codex:gpt-5.5
Update the backfill public API comments now that reply-tree traversal is implemented. Document how documentLoader, maxRequests, maxDepth, and item depth apply to reply targets and replies collections. Assisted-by: Codex:gpt-5.5
Preserve the PR 2 context collection behavior by loading the context collection once for adjacent context strategies. This keeps ordered strategy execution while avoiding duplicate context dereferences and request-budget regressions for explicit context strategy combinations. Add a regression test that combined context object and activity strategies share the same context collection load. Assisted-by: Codex:gpt-5.5
Describe how reply-tree composes with context collection backfill and clarify that reply-tree yields discovered post-like objects without unwrapping Activity objects. Assisted-by: Codex:gpt-5.5
Separate reply-tree object and collection visited state so traversal can avoid reloading or revisiting replies collections. Mark collection IRIs before loading them and keep embedded collection references in visited state to avoid reply-tree collection cycles. Add coverage for repeated replies collection IRIs sharing a single loader request. Assisted-by: Codex:gpt-5.5
Add tests that exercise shared maxItems, maxRequests, and abort behavior across context collection and reply-tree strategies. This protects the ordered hybrid execution path where context-auto runs before reply-tree. Assisted-by: Codex:gpt-5.5
Add a regression test for hybrid backfill ordering where reply-tree runs before context-auto. The test protects the contract that earlier strategies keep their BackfillItem metadata when later strategies discover the same object. Assisted-by: Codex:gpt-5.5
Clarify that backfill strategies run in order while sharing item, request, abort, and deduplication state. Also update the README to describe the opt-in reply-tree strategy alongside the FEP-f228 context collection path. Assisted-by: Codex:gpt-5.5
Limit context-auto absorption to the current adjacent context strategy group so reply-tree keeps acting as an ordering boundary. Add a regression test for context-objects before reply-tree followed by context-auto. Assisted-by: Codex:gpt-5.5
Avoid dereferencing context collection URL items whose object IDs are already known. This prevents fetching the seed object again when a context collection includes it and preserves request budget for unseen items. Assisted-by: Codex:gpt-5.5
Keep a traversal-local document cache in the shared request budget so overlapping strategies do not dereference the same IRI more than once. Cache hits bypass request budgeting and interval delays while thrown loader failures remain retryable. Assisted-by: Codex:gpt-5.5
Add a regression test proving failed documentLoader calls are removed from the traversal-local cache. A later strategy can retry the same IRI and yield the recovered object. Assisted-by: Codex:gpt-5.5
Start descendant traversal from discovered ancestors as well as the seed so reply-tree backfill does not miss sibling branches when the seed is itself a reply. Also skip already visited reply IRIs before dereferencing collection items so bounded crawls do not waste request budget on known objects. Add regressions covering both behaviors. Assisted-by: Codex:gpt-5.5
Apply a default maximum depth of 10 to ancestor and descendant reply-tree traversal while preserving explicit maxDepth overrides. Document the default and cover both traversal directions with tests. Assisted-by: Codex:gpt-5.4
Keep descendant depths relative to the original seed when walking from discovered ancestors, so maxDepth applies consistently across the tree. Allow replies collections to be retried after transient loader failures by marking their IDs visited only after a successful load. Add regression tests for both edge cases. Assisted-by: Codex:gpt-5.4
List Backfilling conversations among the FEPs supported by Fedify so FEDERATION.md reflects the new backfill package capabilities. Assisted-by: Codex:gpt-5.4
Add reply-tree backfill strategy
Add changes and package descriptions. Fix version to 2.3.0. Assisted-by: Codex:gpt-5.5
Publish the @fedify/backfill guide on the documentation website with installation commands, strategy examples, traversal controls, and cache and failure behavior. Add the package to the docs workspace so Twoslash can validate the examples. Assisted-by: Codex:gpt-5.5
Finalize backfill package documentation
❌ Deploy Preview for fedify-json-schema failed.
|
📝 WalkthroughWalkthroughIntroduces the new Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant backfill
participant documentLoader as documentLoader (BackfillContext)
participant APServer as Remote AP Server
App->>backfill: backfill(context, note, { strategies: ["context-auto", "reply-tree"] })
Note over backfill: normalizeStrategies(), init request budget
backfill->>documentLoader: load note.contextId (context collection)
documentLoader->>APServer: GET context IRI
APServer-->>documentLoader: OrderedCollection
documentLoader-->>backfill: collection items
loop each item in collection
backfill->>documentLoader: fetch item IRI (cached)
documentLoader->>APServer: GET object IRI (if not cached)
APServer-->>documentLoader: APObject
backfill-->>App: yield BackfillItem { origin: "collection" }
end
Note over backfill: reply-tree strategy
backfill->>documentLoader: fetch inReplyTo target
documentLoader->>APServer: GET ancestor IRI
APServer-->>documentLoader: APObject
backfill-->>App: yield BackfillItem { origin: "in-reply-to" }
backfill->>documentLoader: fetch replies collection
documentLoader->>APServer: GET replies IRI
APServer-->>documentLoader: Collection
backfill-->>App: yield BackfillItem { origin: "replies" }
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
... and 2 files with indirect coverage changes 🚀 New features to boost your workflow:
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces the @fedify/backfill package, which provides ActivityPub conversation backfill support by reconstructing conversations from FEP-f228 context collections or reply relationships. The changes include the core implementation, extensive unit tests, configuration files, and comprehensive documentation. The code review feedback highlights two key improvement opportunities: replacing Array.prototype.toReversed() with a standard reverse loop to maintain compatibility with Node.js 18, and adding support for the weeks field in durationToMilliseconds to prevent silent evaluation failures when week-based intervals are specified.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Comment on lines
+295
to
+303
| for (const ancestor of ancestors.toReversed()) { | ||
| yield* getReplyDescendants(context, ancestor.object, options, budget, { | ||
| depth: ancestor.depth + 1, | ||
| visitedObjectIds, | ||
| visitedObjects, | ||
| visitedCollectionIds, | ||
| visitedCollections, | ||
| }); | ||
| } |
Contributor
There was a problem hiding this comment.
The use of Array.prototype.toReversed() requires ES2023 support (Node.js 20+). To maintain compatibility with Node.js 18 (which is still a widely used LTS version), we should use a standard reverse loop or [...ancestors].reverse(). A reverse loop is preferred as it avoids creating a shallow copy of the array.
for (let i = ancestors.length - 1; i >= 0; i--) {
const ancestor = ancestors[i];
yield* getReplyDescendants(context, ancestor.object, options, budget, {
depth: ancestor.depth + 1,
visitedObjectIds,
visitedObjects,
visitedCollectionIds,
visitedCollections,
});
}References
- When iterating over an array in reverse, prefer an indexed
forloop over creating a reversed copy with[...items].reverse()if performance and memory allocation are critical.
Comment on lines
+674
to
+680
| return ( | ||
| (duration.milliseconds ?? 0) + | ||
| (duration.seconds ?? 0) * 1000 + | ||
| (duration.minutes ?? 0) * 60 * 1000 + | ||
| (duration.hours ?? 0) * 60 * 60 * 1000 + | ||
| (duration.days ?? 0) * 24 * 60 * 60 * 1000 | ||
| ); |
Contributor
There was a problem hiding this comment.
The durationToMilliseconds function converts Temporal.DurationLike objects to milliseconds without using the global Temporal API. However, it currently ignores the weeks field. If a user specifies an interval in weeks (e.g., { weeks: 1 }), it will silently evaluate to 0 milliseconds. We should add support for the weeks field (1 week = 7 days).
return (
(duration.milliseconds ?? 0) +
(duration.seconds ?? 0) * 1000 +
(duration.minutes ?? 0) * 60 * 1000 +
(duration.hours ?? 0) * 60 * 60 * 1000 +
(duration.days ?? 0) * 24 * 60 * 60 * 1000 +
(duration.weeks ?? 0) * 7 * 24 * 60 * 60 * 1000
);
dahlia
requested changes
Jun 24, 2026
dahlia
left a comment
dahlia
left a comment
Member
There was a problem hiding this comment.
Lockfiles seem outdated; could you please run mise run install to refresh Deno/pnpm lockfiles?
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/backfill/README.md`:
- Around line 48-63: The primary usage example in the README references `note`
without defining it, so update the snippet around `backfill` to include a
concrete `note` declaration before it is passed in. Use the existing
`documentLoader`, `backfill`, and `lookupObject` example context so the snippet
is copy/pasteable and self-contained.
In `@packages/backfill/src/backfill.ts`:
- Around line 62-68: The backfill generator is inferring the yielded item type
from the seed note subtype, which is too narrow for sources like context-auto,
context-activities, and reply-tree. Update backfill and its related types so the
output generic is not driven by the note parameter in backfill(context, note),
and remove the unused TObject from BackfillOptions in types.ts so callers do not
get a misleading homogeneous result type. Keep the existing as TObject cast in
backfill as the API-boundary convention, but adjust the generic contract around
backfill and BackfillOptions so yielded BackfillItem values can represent other
APObject subtypes.
- Around line 660-680: durationToMilliseconds currently drops unsupported
Temporal.DurationLike fields on object inputs, so values like weeks,
microseconds, and nanoseconds can silently become 0 and break throttling. Update
durationToMilliseconds to either convert all supported units used by
Temporal.DurationLike, including weeks/microseconds/nanoseconds, or explicitly
detect unsupported fields and throw in the same helper instead of ignoring them.
Use the durationToMilliseconds function in backfill.ts as the fix point and keep
the string path behavior consistent with the object path.
In `@packages/backfill/tsdown.config.ts`:
- Around line 19-30: The test build in tsdown.config.ts is emitting both ESM and
CJS, which causes the same suites to be picked up twice by the recursive test
runners. Update the defineConfig entry for the src/**/*.test.ts bundle to
produce only one format, or otherwise adjust the test build output so that the
package.json test commands only discover a single extension; use the existing
defineConfig and outExtensions setup to keep the change localized.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9929b7c7-c4ca-445d-8728-25c56641a2b7
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (17)
CHANGES.mdCONTRIBUTING.mdFEDERATION.mddeno.jsondocs/.vitepress/config.mtsdocs/manual/backfill.mddocs/package.jsonpackages/backfill/README.mdpackages/backfill/deno.jsonpackages/backfill/package.jsonpackages/backfill/src/backfill.test.tspackages/backfill/src/backfill.tspackages/backfill/src/mod.tspackages/backfill/src/types.tspackages/backfill/tsdown.config.tspackages/fedify/README.mdpnpm-workspace.yaml
Comment on lines
+48
to
+63
| ~~~~ typescript | ||
| import { backfill } from "@fedify/backfill"; | ||
| import { lookupObject } from "@fedify/vocab"; | ||
|
|
||
| const documentLoader = (iri: URL, options?: { signal?: AbortSignal }) => | ||
| lookupObject(iri, { signal: options?.signal }); | ||
|
|
||
| for await ( | ||
| const item of backfill({ documentLoader }, note, { | ||
| maxItems: 20, | ||
| maxRequests: 50, | ||
| }) | ||
| ) { | ||
| console.log(item.id?.href); | ||
| } | ||
| ~~~~ |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Define note in the primary usage snippet.
The example uses note but never declares it, so copy/paste users get an immediate error.
Proposed fix
import { backfill } from "`@fedify/backfill`";
-import { lookupObject } from "`@fedify/vocab`";
+import { lookupObject, Note } from "`@fedify/vocab`";
+
+declare const note: Note;
const documentLoader = (iri: URL, options?: { signal?: AbortSignal }) =>
lookupObject(iri, { signal: options?.signal });📝 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.
Suggested change
| ~~~~ typescript | |
| import { backfill } from "@fedify/backfill"; | |
| import { lookupObject } from "@fedify/vocab"; | |
| const documentLoader = (iri: URL, options?: { signal?: AbortSignal }) => | |
| lookupObject(iri, { signal: options?.signal }); | |
| for await ( | |
| const item of backfill({ documentLoader }, note, { | |
| maxItems: 20, | |
| maxRequests: 50, | |
| }) | |
| ) { | |
| console.log(item.id?.href); | |
| } | |
| ~~~~ | |
| ~~~~ typescript | |
| import { backfill } from "`@fedify/backfill`"; | |
| import { lookupObject, Note } from "`@fedify/vocab`"; | |
| declare const note: Note; | |
| const documentLoader = (iri: URL, options?: { signal?: AbortSignal }) => | |
| lookupObject(iri, { signal: options?.signal }); | |
| for await ( | |
| const item = backfill({ documentLoader }, note, { | |
| maxItems: 20, | |
| maxRequests: 50, | |
| }) | |
| ) { | |
| console.log(item.id?.href); | |
| } | |
| ~~~~ |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/backfill/README.md` around lines 48 - 63, The primary usage example
in the README references `note` without defining it, so update the snippet
around `backfill` to include a concrete `note` declaration before it is passed
in. Use the existing `documentLoader`, `backfill`, and `lookupObject` example
context so the snippet is copy/pasteable and self-contained.
Comment on lines
+62
to
+68
| export async function* backfill< | ||
| TObject extends APObject = APObject, | ||
| >( | ||
| context: BackfillContext, | ||
| note: TObject, | ||
| options: BackfillOptions<TObject> = {}, | ||
| ): AsyncGenerator<BackfillItem<TObject>, void, void> { |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Decouple the yielded object type from the seed object's subtype.
backfill(context, note) currently infers TObject from note, so a Note seed makes every yielded item a BackfillItem<Note>. That is too narrow here: context-auto, context-activities, and reply-tree can all surface other post-like APObject subtypes. The cast on Line 124 is fine for this repo's API boundary pattern; the problem is that the generic is inferred from the wrong input.
Suggested direction
export async function* backfill<
TObject extends APObject = APObject,
>(
context: BackfillContext,
- note: TObject,
- options: BackfillOptions<TObject> = {},
+ note: APObject,
+ options: BackfillOptions = {},
): AsyncGenerator<BackfillItem<TObject>, void, void> {Also remove the unused TObject parameter from BackfillOptions in packages/backfill/src/types.ts so callers do not get a misleading homogeneous result type through inference.
Based on learnings, the as TObject cast itself matches the repo's vocabulary API-boundary convention; the issue is the generic being inferred from the seed parameter rather than from a separate output contract.
Also applies to: 123-124
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/backfill/src/backfill.ts` around lines 62 - 68, The backfill
generator is inferring the yielded item type from the seed note subtype, which
is too narrow for sources like context-auto, context-activities, and reply-tree.
Update backfill and its related types so the output generic is not driven by the
note parameter in backfill(context, note), and remove the unused TObject from
BackfillOptions in types.ts so callers do not get a misleading homogeneous
result type. Keep the existing as TObject cast in backfill as the API-boundary
convention, but adjust the generic contract around backfill and BackfillOptions
so yielded BackfillItem values can represent other APObject subtypes.
Source: Learnings
Comment on lines
+660
to
+680
| function durationToMilliseconds( | ||
| duration: Temporal.DurationLike | string, | ||
| ): number { | ||
| if (typeof duration === "string") { | ||
| if (typeof Temporal === "undefined") { | ||
| throw new TypeError( | ||
| "Temporal is not globally available; pass interval as a " + | ||
| "Temporal.DurationLike object instead of a string, or provide a " + | ||
| "Temporal polyfill.", | ||
| ); | ||
| } | ||
| return Temporal.Duration.from(duration).total({ unit: "milliseconds" }); | ||
| } | ||
|
|
||
| return ( | ||
| (duration.milliseconds ?? 0) + | ||
| (duration.seconds ?? 0) * 1000 + | ||
| (duration.minutes ?? 0) * 60 * 1000 + | ||
| (duration.hours ?? 0) * 60 * 60 * 1000 + | ||
| (duration.days ?? 0) * 24 * 60 * 60 * 1000 | ||
| ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the target file and inspect the relevant range.
git ls-files 'packages/backfill/src/backfill.ts'
wc -l packages/backfill/src/backfill.ts
sed -n '620,720p' packages/backfill/src/backfill.ts
# Find any other duration conversion helpers/usages for context.
rg -n "durationToMilliseconds|Temporal.DurationLike|weeks|microseconds|nanoseconds" packages/backfill/src/backfill.ts packages/backfill/src -SRepository: fedify-dev/fedify
Length of output: 3508
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the type that feeds durationToMilliseconds().
sed -n '100,140p' packages/backfill/src/types.ts
# Check whether there are any guards or docs indicating supported duration fields.
rg -n "interval|DurationLike|weeks|microseconds|nanoseconds|reject" packages/backfill/src -SRepository: fedify-dev/fedify
Length of output: 2945
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the interval type definition and any surrounding docs.
sed -n '112,132p' packages/backfill/src/types.tsRepository: fedify-dev/fedify
Length of output: 719
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect tests around interval handling to see what shapes are exercised.
sed -n '1720,1795p' packages/backfill/src/backfill.test.ts
# Search for any duration examples in tests or docs.
rg -n 'P1W|weeks:|microseconds:|nanoseconds:|interval:' packages/backfill/src/backfill.test.ts packages/backfill/src -SRepository: fedify-dev/fedify
Length of output: 1952
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1720,1795p' packages/backfill/src/backfill.test.ts
rg -n 'P1W|weeks:|microseconds:|nanoseconds:|interval:' packages/backfill/src/backfill.test.ts packages/backfill/src -SRepository: fedify-dev/fedify
Length of output: 1952
Handle all Temporal.DurationLike fields or reject unsupported ones.
durationToMilliseconds() ignores weeks, microseconds, and nanoseconds on object inputs, so values like { weeks: 1 } collapse to 0 and disable throttling. Either include these units or fail fast instead of silently dropping them.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/backfill/src/backfill.ts` around lines 660 - 680,
durationToMilliseconds currently drops unsupported Temporal.DurationLike fields
on object inputs, so values like weeks, microseconds, and nanoseconds can
silently become 0 and break throttling. Update durationToMilliseconds to either
convert all supported units used by Temporal.DurationLike, including
weeks/microseconds/nanoseconds, or explicitly detect unsupported fields and
throw in the same helper instead of ignoring them. Use the
durationToMilliseconds function in backfill.ts as the fix point and keep the
string path behavior consistent with the object path.
Comment on lines
+19
to
+30
| defineConfig({ | ||
| entry: (await Array.fromAsync(glob(`src/**/*.test.ts`))) | ||
| .map((f) => f.replaceAll(sep, "/")), | ||
| format: ["esm", "cjs"], | ||
| platform: "node", | ||
| outExtensions({ format }) { | ||
| return { | ||
| js: format === "cjs" ? ".cjs" : ".js", | ||
| dts: format === "cjs" ? ".d.cts" : ".d.ts", | ||
| }; | ||
| }, | ||
| deps: { neverBundle: [/^node:/, "@fedify/vocab"] }, |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '== files ==\n'
git ls-files 'packages/backfill/tsdown.config.ts' 'packages/backfill/package.json'
printf '\n== tsdown config outline ==\n'
ast-grep outline packages/backfill/tsdown.config.ts --view expanded || true
printf '\n== relevant tsdown config lines ==\n'
cat -n packages/backfill/tsdown.config.ts | sed -n '1,220p'
printf '\n== package.json lines ==\n'
cat -n packages/backfill/package.json | sed -n '1,220p'Repository: fedify-dev/fedify
Length of output: 3540
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '== files ==\n'
git ls-files 'packages/backfill/tsdown.config.ts' 'packages/backfill/package.json'
printf '\n== tsdown config outline ==\n'
ast-grep outline packages/backfill/tsdown.config.ts --view expanded || true
printf '\n== relevant tsdown config lines ==\n'
cat -n packages/backfill/tsdown.config.ts | sed -n '1,220p'
printf '\n== package.json lines ==\n'
cat -n packages/backfill/package.json | sed -n '1,220p'Repository: fedify-dev/fedify
Length of output: 3540
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '== files ==\n'
git ls-files 'packages/backfill/tsdown.config.ts' 'packages/backfill/package.json'
printf '\n== tsdown config outline ==\n'
ast-grep outline packages/backfill/tsdown.config.ts --view expanded || true
printf '\n== relevant tsdown config lines ==\n'
cat -n packages/backfill/tsdown.config.ts | sed -n '1,220p'
printf '\n== package.json lines ==\n'
cat -n packages/backfill/package.json | sed -n '1,220p'Repository: fedify-dev/fedify
Length of output: 3540
Emit the test bundle in one format. packages/backfill/tsdown.config.ts builds every src/**/*.test.ts twice (esm and cjs), and packages/backfill/package.json runs dist/ recursively with node --test and bun test, so each suite is discovered twice. Either emit one test format here or narrow the test runners to a single extension.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/backfill/tsdown.config.ts` around lines 19 - 30, The test build in
tsdown.config.ts is emitting both ESM and CJS, which causes the same suites to
be picked up twice by the recursive test runners. Update the defineConfig entry
for the src/**/*.test.ts bundle to produce only one format, or otherwise adjust
the test build output so that the package.json test commands only discover a
single extension; use the existing defineConfig and outExtensions setup to keep
the change localized.
Member
Author
I'll reopen the pr |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR merges the completed
feat/backfillfeature branch intomain.The branch adds the initial
@fedify/backfillpackage and completes the FEP-f228 conversation backfill work from the preceding PR series:Closes #275.
Assisted-by: Codex:gpt-5.5