Skip to content

Merge backfill feature branch#820

Merged
dahlia merged 42 commits into
fedify-dev:mainfrom
sij411:feat/backfill
Jun 24, 2026
Merged

Merge backfill feature branch#820
dahlia merged 42 commits into
fedify-dev:mainfrom
sij411:feat/backfill

Conversation

@sij411

@sij411 sij411 commented Jun 24, 2026

Copy link
Copy Markdown
Member

Summary

This PR merges the completed feat/backfill feature branch into main.

The branch adds the initial @fedify/backfill package and completes the FEP-f228 conversation backfill work from the preceding PR series:

  • initial async generator API and context collection object backfill
  • context activity collection support
  • reply-tree traversal and ordered multi-strategy execution
  • package README, manual documentation, changelog, and package-list updates

Closes #275.

Assisted-by: Codex:gpt-5.5

sij411 and others added 30 commits May 23, 2026 11:28
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 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
sij411 and others added 10 commits June 16, 2026 19:13
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 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
@netlify

netlify Bot commented Jun 24, 2026

Copy link
Copy Markdown

Deploy Preview for fedify-json-schema ready!

Name Link
🔨 Latest commit 3d4cc90
🔍 Latest deploy log https://app.netlify.com/projects/fedify-json-schema/deploys/6a3bf038decd1a00081bc1e9
😎 Deploy Preview https://deploy-preview-820--fedify-json-schema.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@coderabbitai

coderabbitai Bot commented Jun 24, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 46c52b8c-c360-4e6c-97cc-7010fcba94e3

📥 Commits

Reviewing files that changed from the base of the PR and between 2fb6371 and 3d4cc90.

📒 Files selected for processing (1)
  • CHANGES.md
💤 Files with no reviewable changes (1)
  • CHANGES.md

📝 Walkthrough

Walkthrough

This PR adds the new @fedify/backfill package for ActivityPub conversation backfilling, with public traversal types, runtime implementation, tests, package wiring, and documentation. It also registers the package in workspace and project metadata.

Changes

@fedify/backfill package introduction

Layer / File(s) Summary
Public types and module entrypoint
packages/backfill/src/types.ts, packages/backfill/src/mod.ts
Defines traversal strategy and origin types, loader/context options, traversal options and result shape, and re-exports the runtime and type API from the package entrypoint.
Backfill traversal implementation
packages/backfill/src/backfill.ts
Implements backfill() with strategy normalization, shared request budgeting, per-IRI caching, context collection traversal, reply-tree traversal, request pacing, cancellation handling, deduplication, and exported error and type-guard helpers.
Package manifests and build configuration
packages/backfill/deno.json, packages/backfill/package.json, packages/backfill/tsdown.config.ts, deno.json, pnpm-workspace.yaml, docs/package.json
Defines the package manifests, build configuration, versioning, scripts, and workspace registration for the new package.
Backfill test suite
packages/backfill/src/backfill.test.ts
Adds tests covering missing and non-collection context, context strategies, reply-tree traversal, deduplication, shared request and item budgets, caching behavior, abort propagation, and interval callback sequencing.
Documentation and project metadata
docs/manual/backfill.md, packages/backfill/README.md, CHANGES.md, CONTRIBUTING.md, FEDERATION.md, docs/.vitepress/config.mts, packages/fedify/README.md
Adds the manual page, package README, changelog entry, federation support entry, VitePress navigation, repository package listing, and contributing workspace directory entry for the new package.

Sequence Diagram(s)

sequenceDiagram
  actor Caller
  participant backfill
  participant RequestBudget
  participant loadObject

  Caller->>backfill: backfill(context, seed, options)
  backfill->>RequestBudget: initialize maxRequests, interval, signal
  loop each selected strategy
    backfill->>loadObject: fetch collection/item/graph document
    loadObject->>RequestBudget: check budget and cache
    loadObject-->>backfill: APObject or null
    backfill-->>Caller: yield BackfillItem
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • fedify-dev/fedify#779: Implements the @fedify/backfill traversal API and supporting types that this PR extends with packaging, docs, and test coverage.
  • fedify-dev/fedify#801: Shares the same @fedify/backfill package area and conversation backfill control flow, including context-based traversal and Create-activity handling.

Suggested labels

component/backfill, component/collections, activitypub/compliance, type/enhancement

Suggested reviewers

  • dahlia
🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title is related, but it is too generic to convey the actual change. Rename it to something specific like 'Add @fedify/backfill conversation backfill package'.
✅ Passed checks (3 passed)
Check name Status Explanation
Description check ✅ Passed The description matches the new backfill package, strategies, docs, and issue closure.
Linked Issues check ✅ Passed The PR adds @fedify/backfill with context, reply-tree, and hybrid traversal plus deduplication and caching, matching #275.
Out of Scope Changes check ✅ Passed The changes are limited to the new package, its docs, workspace wiring, and release notes, with no clear unrelated additions.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces the @fedify/backfill package, which enables the reconstruction of ActivityPub conversations from FEP-f228 context collections or reply relationships. The package includes comprehensive tests, documentation, and support for multiple traversal strategies. Feedback on the implementation highlights opportunities to optimize reverse array iteration by using an indexed for loop instead of .toReversed(), prevent potential runtime crashes by safely accessing note.contextIds with optional chaining, and improve abort handling by providing a fallback DOMException when budget.signal.reason is undefined.

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.

ancestors.push({ object: item.object, depth: item.depth });
yield item;
}
for (const ancestor of ancestors.toReversed()) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

When iterating over an array in reverse, prefer an indexed for loop over creating a reversed copy with [...items].reverse() or .toReversed() if performance and memory allocation are critical.

for (let i = ancestors.length - 1; i >= 0; i--) {
  const ancestor = ancestors[i];
References
  1. When iterating over an array in reverse, prefer an indexed for loop over creating a reversed copy with [...items].reverse() if performance and memory allocation are critical.

readonly origin: "collection";
readonly depth: 0;
}> {
const contextId = note.contextIds[0];

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

To prevent potential runtime TypeError crashes if note.contextIds is undefined or null, it is safer to use optional chaining when accessing the first element.

Suggested change
const contextId = note.contextIds[0];
const contextId = note.contextIds?.[0];

Comment on lines +643 to +657
await new Promise<void>((resolve, reject) => {
if (budget.signal?.aborted) {
reject(budget.signal.reason);
return;
}
const timeout = setTimeout(() => {
budget.signal?.removeEventListener("abort", onAbort);
resolve();
}, milliseconds);
const onAbort = () => {
clearTimeout(timeout);
reject(budget.signal?.reason);
};
budget.signal?.addEventListener("abort", onAbort, { once: true });
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

If budget.signal is aborted without an explicit reason, budget.signal.reason can be undefined in some environments or custom signal implementations. Rejecting a promise with undefined is an anti-pattern and can cause issues in catch blocks. Providing a fallback DOMException ensures robust error handling.

Suggested change
await new Promise<void>((resolve, reject) => {
if (budget.signal?.aborted) {
reject(budget.signal.reason);
return;
}
const timeout = setTimeout(() => {
budget.signal?.removeEventListener("abort", onAbort);
resolve();
}, milliseconds);
const onAbort = () => {
clearTimeout(timeout);
reject(budget.signal?.reason);
};
budget.signal?.addEventListener("abort", onAbort, { once: true });
});
await new Promise<void>((resolve, reject) => {
if (budget.signal?.aborted) {
reject(budget.signal.reason ?? new DOMException("The operation was aborted.", "AbortError"));
return;
}
const timeout = setTimeout(() => {
budget.signal?.removeEventListener("abort", onAbort);
resolve();
}, milliseconds);
const onAbort = () => {
clearTimeout(timeout);
reject(budget.signal?.reason ?? new DOMException("The operation was aborted.", "AbortError"));
};
budget.signal?.addEventListener("abort", onAbort, { once: true });
});

@codecov

codecov Bot commented Jun 24, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 86.24272% with 543 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
packages/cli/src/bench/compare.ts 67.71% 195 Missing and 61 partials ⚠️
packages/cli/src/bench/action.ts 83.51% 81 Missing and 23 partials ⚠️
packages/cli/src/bench/result/build.ts 86.02% 19 Missing and 19 partials ⚠️
packages/cli/src/bench/result/expect/evaluate.ts 79.13% 15 Missing and 14 partials ⚠️
packages/cli/src/bench/metrics/stats-client.ts 89.25% 9 Missing and 14 partials ⚠️
packages/cli/src/bench/load/generator.ts 87.41% 15 Missing and 4 partials ⚠️
packages/cli/src/bench/metrics/histogram.ts 89.06% 5 Missing and 9 partials ⚠️
packages/cli/src/bench/render/text.ts 91.81% 1 Missing and 8 partials ⚠️
packages/cli/src/bench/discovery/discover.ts 86.00% 4 Missing and 3 partials ⚠️
packages/cli/src/bench/result/expect/metrics.ts 78.78% 3 Missing and 4 partials ⚠️
... and 11 more
Files with missing lines Coverage Δ
packages/cli/src/bench/actor/fleet.ts 100.00% <100.00%> (ø)
packages/cli/src/bench/command.ts 100.00% <100.00%> (ø)
packages/cli/src/bench/compare/schema.ts 100.00% <100.00%> (ø)
packages/cli/src/bench/render/index.ts 100.00% <100.00%> (ø)
packages/cli/src/bench/render/json.ts 100.00% <100.00%> (ø)
packages/cli/src/bench/result/expect/assert.ts 100.00% <100.00%> (ø)
packages/cli/src/bench/result/schema.ts 100.00% <100.00%> (ø)
packages/cli/src/bench/safety/gate.ts 100.00% <100.00%> (ø)
packages/cli/src/bench/scenario/coerce.ts 100.00% <100.00%> (ø)
packages/cli/src/bench/scenario/normalize.ts 100.00% <100.00%> (ø)
... and 90 more

... and 20 files with indirect coverage changes

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 `@CHANGES.md`:
- Line 359: The CHANGES markdown contains a duplicate reference definition for
[`#801`], which is already defined earlier and should only appear once. Remove the
later [`#801`] definition in the changelog entry so the existing reference
definition remains the single source of truth and markdown lint no longer flags
the duplicate.

In `@packages/backfill/package.json`:
- Around line 64-66: The backfill test scripts are running both emitted copies
of each suite, which duplicates execution and can cause shared-state flakiness.
Update the test commands in package.json to target only one emitted format from
tsdown.config.ts, and use the matching runner invocation for that single format
in the test:bun/test scripts so the same tests are not discovered twice.
Reference the test script entries in package.json and the build output shape
from tsdown.config.ts when adjusting the glob/command.

In `@packages/backfill/src/backfill.ts`:
- Around line 145-160: The strategy normalizer currently only removes trailing
adjacent context strategies when it sees "context-auto", so non-adjacent context
strategies like "context-objects" can remain and violate the documented
absorption contract. Update the normalization logic in the strategy-processing
loop that checks "context-auto", "isContextStrategy", and
currentContextGroupHasAuto so that "context-auto" absorbs all other context
strategies already present in normalized, not just contiguous ones. Make sure
the resulting behavior matches the contract described in the backfill types
documentation and prevents duplicate context passes from being retained.
- Around line 660-680: The durationToMilliseconds helper is undercounting valid
Temporal.DurationLike inputs because it ignores weeks, microseconds, and
nanoseconds, and it silently drops calendar-based fields like months and years.
Update durationToMilliseconds to convert all supported components consistently
(or delegate to Temporal.Duration.from(...).total(...) for object inputs when
Temporal is available), and make sure the interval pacing logic in backfill.ts
uses the corrected total from durationToMilliseconds.
🪄 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: 1d8e2551-67a3-413b-977f-54911e5e9329

📥 Commits

Reviewing files that changed from the base of the PR and between 275c2d1 and 2fb6371.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (17)
  • CHANGES.md
  • CONTRIBUTING.md
  • FEDERATION.md
  • deno.json
  • docs/.vitepress/config.mts
  • docs/manual/backfill.md
  • docs/package.json
  • packages/backfill/README.md
  • packages/backfill/deno.json
  • packages/backfill/package.json
  • packages/backfill/src/backfill.test.ts
  • packages/backfill/src/backfill.ts
  • packages/backfill/src/mod.ts
  • packages/backfill/src/types.ts
  • packages/backfill/tsdown.config.ts
  • packages/fedify/README.md
  • pnpm-workspace.yaml

Comment thread CHANGES.md Outdated
Comment on lines +64 to +66
"test": "cd dist/ && node --test",
"pretest:bun": "pnpm build",
"test:bun": "cd dist/ && bun test --timeout 60000"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify that the test build emits both module formats while the scripts use dist-wide discovery.
sed -n '19,31p' packages/backfill/tsdown.config.ts
sed -n '58,66p' packages/backfill/package.json

Repository: fedify-dev/fedify

Length of output: 875


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect related package test/build patterns and any dist test discovery hints.
git ls-files 'packages/*/package.json' 'packages/*/tsdown.config.ts' | sed -n '1,200p'
printf '\n--- backfill package.json ---\n'
cat -n packages/backfill/package.json | sed -n '55,75p'
printf '\n--- backfill tsdown.config.ts ---\n'
cat -n packages/backfill/tsdown.config.ts | sed -n '1,80p'
printf '\n--- other package test scripts ---\n'
rg -n '"test": "cd dist/ && (node --test|bun test)' packages -g 'package.json'

Repository: fedify-dev/fedify

Length of output: 4184


Limit backfill tests to one emitted format. packages/backfill/tsdown.config.ts builds each test file as both *.test.js and *.test.cjs, and packages/backfill/package.json runs node --test/bun test over dist/. That duplicates the same suite and can make shared-state assertions flaky.

🤖 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/package.json` around lines 64 - 66, The backfill test
scripts are running both emitted copies of each suite, which duplicates
execution and can cause shared-state flakiness. Update the test commands in
package.json to target only one emitted format from tsdown.config.ts, and use
the matching runner invocation for that single format in the test:bun/test
scripts so the same tests are not discovered twice. Reference the test script
entries in package.json and the build output shape from tsdown.config.ts when
adjusting the glob/command.

Comment on lines +145 to +160
for (const strategy of strategies) {
if (strategy === "context-auto") {
for (
let i = normalized.length - 1;
i >= 0 && isContextStrategy(normalized[i]);
i--
) {
normalized.splice(i, 1);
}
if (!normalized.includes(strategy)) normalized.push(strategy);
} else if (isContextStrategy(strategy)) {
if (
!currentContextGroupHasAuto(normalized) &&
!normalized.includes(strategy)
) {
normalized.push(strategy);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

context-auto only absorbs adjacent context strategies right now.

With ["context-objects", "reply-tree", "context-auto"], this normalizer keeps both context passes, even though the public contract in packages/backfill/src/types.ts Lines 84-86 says "context-auto" absorbs the other context strategies. That changes winner metadata and can spend extra requests unexpectedly. Please make the behavior and the documented contract agree.

🤖 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 145 - 160, The strategy
normalizer currently only removes trailing adjacent context strategies when it
sees "context-auto", so non-adjacent context strategies like "context-objects"
can remain and violate the documented absorption contract. Update the
normalization logic in the strategy-processing loop that checks "context-auto",
"isContextStrategy", and currentContextGroupHasAuto so that "context-auto"
absorbs all other context strategies already present in normalized, not just
contiguous ones. Make sure the resulting behavior matches the contract described
in the backfill types documentation and prevents duplicate context passes from
being retained.

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
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Temporal.DurationLike object inputs are undercounted here.

weeks, microseconds, and nanoseconds are ignored, and calendar units like months/years are silently treated as zero. For example, { weeks: 1 } currently yields no delay, which breaks the pacing contract for valid interval values.

Proposed fix
 function durationToMilliseconds(
   duration: Temporal.DurationLike | string,
 ): number {
   if (typeof duration === "string") {
@@
   }
 
+  if ((duration.years ?? 0) !== 0 || (duration.months ?? 0) !== 0) {
+    throw new RangeError(
+      "interval does not support calendar units without a relative reference.",
+    );
+  }
+
   return (
     (duration.milliseconds ?? 0) +
+    (duration.microseconds ?? 0) / 1000 +
+    (duration.nanoseconds ?? 0) / 1_000_000 +
     (duration.seconds ?? 0) * 1000 +
     (duration.minutes ?? 0) * 60 * 1000 +
     (duration.hours ?? 0) * 60 * 60 * 1000 +
+    (duration.weeks ?? 0) * 7 * 24 * 60 * 60 * 1000 +
     (duration.days ?? 0) * 24 * 60 * 60 * 1000
   );
 }
📝 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
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
);
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" });
}
if ((duration.years ?? 0) !== 0 || (duration.months ?? 0) !== 0) {
throw new RangeError(
"interval does not support calendar units without a relative reference.",
);
}
return (
(duration.milliseconds ?? 0) +
(duration.microseconds ?? 0) / 1000 +
(duration.nanoseconds ?? 0) / 1_000_000 +
(duration.seconds ?? 0) * 1000 +
(duration.minutes ?? 0) * 60 * 1000 +
(duration.hours ?? 0) * 60 * 60 * 1000 +
(duration.weeks ?? 0) * 7 * 24 * 60 * 60 * 1000 +
(duration.days ?? 0) * 24 * 60 * 60 * 1000
);
🤖 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, The
durationToMilliseconds helper is undercounting valid Temporal.DurationLike
inputs because it ignores weeks, microseconds, and nanoseconds, and it silently
drops calendar-based fields like months and years. Update durationToMilliseconds
to convert all supported components consistently (or delegate to
Temporal.Duration.from(...).total(...) for object inputs when Temporal is
available), and make sure the interval pacing logic in backfill.ts uses the
corrected total from durationToMilliseconds.

@dahlia

dahlia commented Jun 24, 2026

Copy link
Copy Markdown
Member
[check:md] /home/runner/work/fedify/fedify/CHANGES.md: not formatted

You should run mise run fmt to format Markdown documents!

@dahlia dahlia changed the base branch from main to feat/backfill June 24, 2026 15:04
dahlia
dahlia previously approved these changes Jun 24, 2026
@dahlia dahlia changed the base branch from feat/backfill to main June 24, 2026 15:08
@dahlia dahlia dismissed their stale review June 24, 2026 15:08

The base branch was changed.

@dahlia dahlia merged commit 5310b12 into fedify-dev:main Jun 24, 2026
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

@fedify/backfill package for FEP-f228 conversation backfill

2 participants