Add property preprocessors to @fedify/vocab-tools and normalize Link icon/image#793
Add property preprocessors to @fedify/vocab-tools and normalize Link icon/image#793dahlia wants to merge 14 commits into
@fedify/vocab-tools and normalize Link icon/image#793Conversation
Added `PropertyPreprocessor` types to `@fedify/vocab-runtime` and `preprocessors` schema support to `@fedify/vocab-tools`. Generated decoders now run property-level preprocessors before the normal range decoder for each expanded JSON-LD property value. A preprocessor receives an expanded JSON-LD value and may return a vocabulary object, `undefined` (fall through to normal decoder), or an `Error` (fail the decode). Module specifiers are resolved from the generated vocabulary source file and imported dynamically. Assisted-by: OpenCode:deepseek-v4-pro
Added `normalizeLinkToImage` preprocessor to `@fedify/vocab` that converts incoming explicit `Link` objects in the `icon` and `image` properties into `Image` objects during decoding. This keeps the public TypeScript API `Image`-oriented while allowing wire-level `Link` values to pass through without errors. The `icon` and `image` properties in `Object` now declare the new preprocessor in their schema definitions via the `preprocessors` field, and the generated decoders run it before the normal range decoder for each expanded JSON-LD value. Regression tests cover single and multiple Link icons, relative URL resolution, and mixed Link+Image arrays. Assisted-by: OpenCode:deepseek-v4-pro
Added changelog entries for @fedify/vocab-runtime, @fedify/vocab, and @fedify/vocab-tools describing the new PropertyPreprocessor types, Link-to-Image normalization, and schema preprocessors field. References: fedify-dev#790 fedify-dev#792 Assisted-by: OpenCode:deepseek-v4-pro
The generated `#<property>_fromJsonLd` helper now expands fetched documents before passing values to preprocessors. This ensures that remote `Link` icons and images resolved via URL dereferencing are normalized to `Image` objects, matching the decoder path's expanded value contract. Added a regression test for URL-backed icons that are fetched as compact Link documents. Assisted-by: OpenCode:deepseek-v4-pro Assisted-by: Codex:gpt-5.5
|
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 schema-declared property preprocessors: runtime types and exports, schema and JSON Schema extensions, generator changes to emit preprocessor invocation in decoders (with module imports), property-level preprocessing logic, a Link→Image preprocessor implementation, and tests validating normalization. ChangesProperty preprocessors feature
Sequence Diagram(s)sequenceDiagram
participant Caller
participant GeneratedDecoder
participant PreprocessorModule
participant RangeDecoder
Caller->>GeneratedDecoder: Object.fromJsonLd(jsonLd, options)
GeneratedDecoder->>PreprocessorModule: preprocessor(v, {documentLoader, contextLoader, tracerProvider, baseUrl})
PreprocessorModule-->>GeneratedDecoder: Image | undefined | Error
alt preprocessor returned Image
GeneratedDecoder->>GeneratedDecoder: push preprocessed Image and continue
else preprocessor returned undefined
GeneratedDecoder->>RangeDecoder: Range.fromJsonLd(v, optionsWithBaseUrl)
RangeDecoder-->>GeneratedDecoder: Image
else preprocessor returned Error
PreprocessorModule-->>Caller: throw Error
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces property preprocessors to normalize wire-level JSON-LD property values before decoding, specifically implementing a preprocessor to normalize ActivityStreams Link objects in icon and image properties to Image. Feedback on the changes highlights two key issues: first, using dynamic import() with hardcoded .ts extensions in the generated decoder loop will cause runtime MODULE_NOT_FOUND errors in Node.js/Bun environments compiled via tsdown, as well as unnecessary async overhead; second, calling new URL() on blank nodes or invalid URLs in values["@id"] can throw a TypeError and crash the decoding process, which can be avoided by validating with URL.canParse first.
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.
|
@codex review |
dahlia
added
component/vocab
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e4794550f1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/vocab-runtime/src/preprocessor.ts`:
- Around line 4-22: Add JSDoc comments for the exported types to document the
public API: describe Json as the serializable JSON value union and give
examples/constraints; document PropertyPreprocessorContext fields
(documentLoader, contextLoader, tracerProvider, baseUrl) and their types/roles;
and document PropertyPreprocessor<T> describing its parameters (value: Json,
context: PropertyPreprocessorContext), return possibilities (T | undefined |
Error | Promise<...>) and when each is used. Attach the JSDoc directly above the
exported symbols Json, PropertyPreprocessorContext, and PropertyPreprocessor in
preprocessor.ts so the comments are included in the published API docs.
In `@packages/vocab-tools/src/schema.yaml`:
- Around line 130-135: The schema allows an empty string for the module
specifier (the "module" property), causing runtime dynamic import failures;
update the "module" property in schema.yaml (the module field used by
preprocessors) to require a non-empty string by adding a JSON Schema constraint
such as minLength: 1 (or a pattern like '^.+$') so empty values fail validation
rather than at runtime.
🪄 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: 67b2a424-7a69-412d-874f-7ac4cd139fed
⛔ Files ignored due to path filters (1)
packages/vocab-tools/src/__snapshots__/class.test.ts.deno.snapis excluded by!**/*.snap
📒 Files selected for processing (12)
CHANGES.mdpackages/vocab-runtime/src/mod.tspackages/vocab-runtime/src/preprocessor.tspackages/vocab-tools/src/class.tspackages/vocab-tools/src/codec.tspackages/vocab-tools/src/mod.tspackages/vocab-tools/src/property.tspackages/vocab-tools/src/schema.tspackages/vocab-tools/src/schema.yamlpackages/vocab/src/object.yamlpackages/vocab/src/preprocessors.tspackages/vocab/src/vocab.test.ts
Codecov Report❌ Patch coverage is
🚀 New features to boost your workflow:
|
dahlia
changed the title
@fedify/vocab-tools and normalize Link icon/image
Regenerated snapshot files for the `generateClasses()` test that now includes preprocessor blocks in the emitted icon/image decoder code. Assisted-by: OpenCode:deepseek-v4-pro
Switch from dynamic import() to static imports for preprocessor modules so the generated code works correctly in Node.js/Bun environments after tsdown compilation. Dynamic imports with .ts extensions are not rewritten by transpilers and cause runtime MODULE_NOT_FOUND errors. Also fix: - Blank node IDs (e.g. _:b0) no longer crash the decoder - Cached property reparse passes the parent baseUrl for relative URL resolution in Link-derived icon/image values - Schema now requires non-empty module specifiers - JSDoc comments added to public preprocessor types fedify-dev#793 (comment) fedify-dev#793 (comment) fedify-dev#793 (comment) fedify-dev#793 (comment) fedify-dev#793 (comment) Assisted-by: OpenCode:deepseek-v4-pro
|
@codex review |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces property preprocessing to the vocabulary generation toolchain, allowing wire-level JSON-LD property values to be normalized before decoding. Specifically, it adds PropertyPreprocessor types to @fedify/vocab-runtime, updates @fedify/vocab-tools to support a preprocessors field in schemas, and implements a normalizeLinkToImage preprocessor in @fedify/vocab to normalize ActivityStreams Link objects to Image objects for icon and image properties. The reviewer noted that in packages/vocab-tools/src/codec.ts, relative @ids will not be resolved correctly because URL.canParse and new URL are called without options.baseUrl as the second argument.
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.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bbfe8d0a7e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/vocab-tools/src/codec.ts (2)
401-403:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMissing
URL.canParseguard when settingbaseUrlfrom@id.If
values["@id"]is a blank node identifier (e.g.,_:b0) or otherwise not a valid URL,new URL(values["@id"])will throw. This should use the same guard pattern as lines 476 and 500.Proposed fix
- if (options.baseUrl == null && values["`@id`"] != null) { - options = { ...options, baseUrl: new URL(values["`@id`"]) }; + if (options.baseUrl == null && values["`@id`"] != null && + URL.canParse(values["`@id`"])) { + options = { ...options, baseUrl: new URL(values["`@id`"]) }; }🤖 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/vocab-tools/src/codec.ts` around lines 401 - 403, The code sets options.baseUrl from values["`@id`"] without validating it, causing new URL(values["`@id`"]) to throw for blank-node IDs; update the block in codec.ts (the branch that assigns options = { ...options, baseUrl: new URL(values["`@id`"]) }) to first check URL.canParse(values["`@id`"]) (same guard pattern used at the locations around lines 476 and 500) and only construct new URL(...) when that guard returns true, otherwise leave options.baseUrl unchanged.
509-515:⚠️ Potential issue | 🟠 Major | ⚡ Quick winInconsistent
baseUrlfallback: missingURL.canParseguard.Lines 476 and 500 use
(values["@id"] == null || !URL.canParse(values["@id"]) ? options.baseUrl : new URL(values["@id"]))to safely handle non-parseable@idvalues, but line 514 omits theURL.canParsecheck. Ifvalues["@id"]is present but not a valid URL (e.g., a blank node identifier like_:b0),new URL(values["@id"])will throw.Proposed fix
const decoders = getDecoders( property.range, types, "v", "options", - `(values["`@id`"] == null ? options.baseUrl : new URL(values["`@id`"]))`, + `(values["`@id`"] == null || !URL.canParse(values["`@id`"]) ? options.baseUrl : new URL(values["`@id`"]))`, );🤖 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/vocab-tools/src/codec.ts` around lines 509 - 515, The call to getDecoders is using new URL(values["`@id`"]) without the URL.canParse guard, so malformed `@id` values will throw; update the argument passed as the base URL (the fifth parameter in the getDecoders(...) call) to match the safe pattern used elsewhere: use (values["`@id`"] == null || !URL.canParse(values["`@id`"]) ? options.baseUrl : new URL(values["`@id`"])) so that getDecoders receives options.baseUrl when values["`@id`"] is missing or not a parseable URL.
🤖 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.
Outside diff comments:
In `@packages/vocab-tools/src/codec.ts`:
- Around line 401-403: The code sets options.baseUrl from values["`@id`"] without
validating it, causing new URL(values["`@id`"]) to throw for blank-node IDs;
update the block in codec.ts (the branch that assigns options = { ...options,
baseUrl: new URL(values["`@id`"]) }) to first check URL.canParse(values["`@id`"])
(same guard pattern used at the locations around lines 476 and 500) and only
construct new URL(...) when that guard returns true, otherwise leave
options.baseUrl unchanged.
- Around line 509-515: The call to getDecoders is using new URL(values["`@id`"])
without the URL.canParse guard, so malformed `@id` values will throw; update the
argument passed as the base URL (the fifth parameter in the getDecoders(...)
call) to match the safe pattern used elsewhere: use (values["`@id`"] == null ||
!URL.canParse(values["`@id`"]) ? options.baseUrl : new URL(values["`@id`"])) so that
getDecoders receives options.baseUrl when values["`@id`"] is missing or not a
parseable URL.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: b35aa6ac-3b0f-4096-b5eb-506301789907
⛔ Files ignored due to path filters (3)
packages/vocab-tools/src/__snapshots__/class.test.ts.deno.snapis excluded by!**/*.snappackages/vocab-tools/src/__snapshots__/class.test.ts.node.snapis excluded by!**/*.snappackages/vocab-tools/src/__snapshots__/class.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (5)
packages/vocab-runtime/src/preprocessor.tspackages/vocab-tools/src/class.tspackages/vocab-tools/src/codec.tspackages/vocab-tools/src/property.tspackages/vocab-tools/src/schema.yaml
Two additional codec.ts branches constructed new URL(values["@id"]) without the URL.canParse guard: the options.baseUrl fallback and the getDecoders base URL argument. Both now use the same safe pattern as the preprocessor and getDecoder blocks so blank-node identifiers (e.g. _:b0) do not crash the decoder. Assisted-by: OpenCode:deepseek-v4-pro
Pass options.baseUrl as the second argument to URL.canParse() and new URL() so that relative @id values (e.g. "notes/1") are correctly resolved rather than falling back to the document base URL. Also regenerated all vocabulary code generation snapshots to reflect the cached property reparse baseUrl change. fedify-dev#793 (comment) fedify-dev#793 (comment) fedify-dev#793 (comment) Assisted-by: OpenCode:deepseek-v4-pro
|
@codex review |
|
/gemini review |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/vocab-tools/src/codec.ts (2)
33-33: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winConsider throwing an error when preprocessor module is not found.
Line 33 silently skips a preprocessor if its module variable is not found in
moduleVarNames. This could hide a generator bug whereclass.tsfailed to import a required preprocessor module. The schema declares preprocessors that won't be invoked, potentially leading to incorrect decoder behavior.Consider throwing a descriptive error during code generation to catch configuration mismatches early.
Suggested improvement
for (const pp of property.preprocessors) { const varName = moduleVarNames.get(pp.module); - if (varName == null) continue; + if (varName == null) { + throw new Error( + `Preprocessor module "${pp.module}" not found in moduleVarNames. ` + + `Ensure class generator imports all required preprocessor modules.` + ); + }🤖 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/vocab-tools/src/codec.ts` at line 33, Replace the silent skip on missing preprocessor module vars by throwing a descriptive error: in packages/vocab-tools/src/codec.ts where you check moduleVarNames and hit the branch "if (varName == null) continue;", change that behavior to throw an Error that includes the preprocessor name (from the current preprocessor entry), the expected module variable key, and guidance that the module import is missing (so codegen fails fast). Update any surrounding logic in the function that iterates preprocessors (the loop handling "preprocessors" and the lookup into "moduleVarNames") to propagate/handle the thrown error as needed.
476-476: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winExtract duplicated baseUrl expression to improve maintainability.
The baseUrl expression is duplicated three times across lines 476, 500, and 514:
(values["`@id`"] == null || !URL.canParse(values["`@id`"]) ? options.baseUrl : new URL(values["`@id`"]))This PR adds two new instances (lines 476, 514) on top of the existing one (line 500). The complex expression should be extracted to a constant to establish a single source of truth and simplify future maintenance.
♻️ Proposed refactor
for (const property of type.properties) { const variable = await getFieldName(property.uri, ""); yield await generateField(property, types, "const "); + const baseUrlExpr = `(values["`@id`"] == null || !URL.canParse(values["`@id`"]) ? options.baseUrl : new URL(values["`@id`"]))`; const arrayVariable = `${variable}__array`; yield ` let ${arrayVariable} = values[${JSON.stringify(property.uri)}]; ... yield* generatePreprocessorBlock( property, getTypeNames(property.range, types), variable, - `(values["`@id`"] == null || !URL.canParse(values["`@id`"]) ? options.baseUrl : new URL(values["`@id`"]))`, + baseUrlExpr, moduleVarNames, ); ... if (property.range.length == 1) { yield ` const decoded = ${ getDecoder( property.range[0], types, "v", "options", - `(values["`@id`"] == null || !URL.canParse(values["`@id`"]) ? options.baseUrl : new URL(values["`@id`"]))`, + baseUrlExpr, ) }; ... const decoders = getDecoders( property.range, types, "v", "options", - `(values["`@id`"] == null || !URL.canParse(values["`@id`"]) ? options.baseUrl : new URL(values["`@id`"]))`, + baseUrlExpr, );Also applies to: 500-500, 514-514
🤖 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/vocab-tools/src/codec.ts` at line 476, The duplicated complex baseUrl selection expression (values["`@id`"] == null || !URL.canParse(values["`@id`"]) ? options.baseUrl : new URL(values["`@id`"])) should be extracted to a single local constant (e.g., resolvedBaseUrl or parsedIdUrl) and the three occurrences replaced with that constant; locate the uses around the codec logic that reference values["`@id`"], options.baseUrl and URL.canParse and compute the constant once before those uses so future changes only need a single edit.
🤖 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.
Outside diff comments:
In `@packages/vocab-tools/src/codec.ts`:
- Line 33: Replace the silent skip on missing preprocessor module vars by
throwing a descriptive error: in packages/vocab-tools/src/codec.ts where you
check moduleVarNames and hit the branch "if (varName == null) continue;", change
that behavior to throw an Error that includes the preprocessor name (from the
current preprocessor entry), the expected module variable key, and guidance that
the module import is missing (so codegen fails fast). Update any surrounding
logic in the function that iterates preprocessors (the loop handling
"preprocessors" and the lookup into "moduleVarNames") to propagate/handle the
thrown error as needed.
- Line 476: The duplicated complex baseUrl selection expression (values["`@id`"]
== null || !URL.canParse(values["`@id`"]) ? options.baseUrl : new
URL(values["`@id`"])) should be extracted to a single local constant (e.g.,
resolvedBaseUrl or parsedIdUrl) and the three occurrences replaced with that
constant; locate the uses around the codec logic that reference values["`@id`"],
options.baseUrl and URL.canParse and compute the constant once before those uses
so future changes only need a single edit.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4c5b1e02-f558-4bd4-8f24-d04c6eb02321
📒 Files selected for processing (1)
packages/vocab-tools/src/codec.ts
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Code Review
This pull request introduces property preprocessors to the vocabulary generation and runtime system. It adds 'PropertyPreprocessor', 'PropertyPreprocessorContext', and 'Json' types to '@fedify/vocab-runtime' to allow normalizing wire-level JSON-LD property values before range decoders run. In '@fedify/vocab-tools', property schemas now support a 'preprocessors' field specifying module/function pairs, and the generated decoders are updated to dynamically import and execute these preprocessors. Finally, in '@fedify/vocab', the 'normalizeLinkToImage' preprocessor is added to normalize ActivityStreams 'Link' objects in 'icon' and 'image' properties to 'Image' objects during decoding, accompanied by comprehensive unit tests. No review comments were provided, so there is no additional feedback to address.
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.
Replace silent skip with a descriptive error when a preprocessor module is not in the generated import map, so missing modules are caught at codegen time rather than silently at runtime. Extract the repeated baseUrl selection expression into a single `_baseUrl` constant computed once per decode invocation, used by the preprocessor block, getDecoder, and getDecoders call sites. Assisted-by: OpenCode:deepseek-v4-pro
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/vocab-tools/src/codec.ts (1)
401-403:⚠️ Potential issue | 🟠 Major | ⚡ Quick winResolve every
@idthrough one shared base URL.These new expressions only feed preprocessors and
getDecoder(s). Line 429 and Lines 484-486 still callnew URL(...)with the raw@id, so a relative root@idor an id-only reference like{"@id":"notes/1"}still throws even whenoptions.baseUrlis set. Compute the resolved base once and reuse it in all URL construction paths.Suggested fix
- if (options.baseUrl == null && values["`@id`"] != null && URL.canParse(values["`@id`"])) { - options = { ...options, baseUrl: new URL(values["`@id`"]) }; - } + const resolvedBaseUrl = + values["`@id`"] == null || + !URL.canParse(values["`@id`"], options.baseUrl) + ? options.baseUrl + : new URL(values["`@id`"], options.baseUrl); + if (options.baseUrl == null && resolvedBaseUrl != null) { + options = { ...options, baseUrl: resolvedBaseUrl }; + }Then reuse
resolvedBaseUrlin the remaining direct URL branches too:- { id: "`@id`" in values ? new URL(values["`@id`"] as string) : undefined }, + { id: "`@id`" in values ? new URL(values["`@id`"] as string, resolvedBaseUrl) : undefined }, - !URL.canParse(v["`@id`"]) && v["`@id`"].startsWith("at://") + !URL.canParse(v["`@id`"], resolvedBaseUrl) && v["`@id`"].startsWith("at://") ? new URL("at://" + encodeURIComponent(v["`@id`"].substring(5))) - : new URL(v["`@id`"]) + : new URL(v["`@id`"], resolvedBaseUrl)And pass
resolvedBaseUrlinto the preprocessor / decoder helpers instead of repeating the inline expression.Also applies to: 472-515
🤖 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/vocab-tools/src/codec.ts` around lines 401 - 403, Compute a single resolvedBaseUrl once (e.g., when options.baseUrl is null and values["`@id`"] is present) and use that resolvedBaseUrl everywhere instead of calling new URL(values["`@id`"]) inline; update the branches around the inline new URL calls (the branches referenced near lines 429, 472-515, and 484-486) to use this resolvedBaseUrl, and pass resolvedBaseUrl into the preprocessor and getDecoder/getDecoders helper calls (and any other call sites that currently call new URL(values["`@id`"])) so all URL resolution is centralized and consistent.
🤖 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.
Outside diff comments:
In `@packages/vocab-tools/src/codec.ts`:
- Around line 401-403: Compute a single resolvedBaseUrl once (e.g., when
options.baseUrl is null and values["`@id`"] is present) and use that
resolvedBaseUrl everywhere instead of calling new URL(values["`@id`"]) inline;
update the branches around the inline new URL calls (the branches referenced
near lines 429, 472-515, and 484-486) to use this resolvedBaseUrl, and pass
resolvedBaseUrl into the preprocessor and getDecoder/getDecoders helper calls
(and any other call sites that currently call new URL(values["`@id`"])) so all URL
resolution is centralized and consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2823c7d1-7e7e-49be-9fcf-4698e3628b0d
⛔ Files ignored due to path filters (3)
packages/vocab-tools/src/__snapshots__/class.test.ts.deno.snapis excluded by!**/*.snappackages/vocab-tools/src/__snapshots__/class.test.ts.node.snapis excluded by!**/*.snappackages/vocab-tools/src/__snapshots__/class.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (1)
packages/vocab-tools/src/codec.ts
Replace two separate new URL() calls with a single _resolvedUrl computed from values["@id"] against options.baseUrl. The result flows into both the options.baseUrl fallback assignment and the _baseUrl constant used by all downstream decode logic, eliminating duplication and ensuring consistent resolution with the base URL. Assisted-by: OpenCode:deepseek-v4-pro
Added `@since 2.3.0` tags to `Json`, `PropertyPreprocessorContext`, and `PropertyPreprocessor<T>` in @fedify/vocab-runtime, and added full JSDoc with @SInCE tag to `normalizeLinkToImage`. Assisted-by: OpenCode:deepseek-v4-pro
Assisted-by: OpenCode:deepseek-v4-pro
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces property preprocessors to the vocabulary generation toolchain and runtime, allowing wire-level JSON-LD property values to be normalized before decoding. Specifically, it implements a normalizeLinkToImage preprocessor to convert explicit ActivityStreams Link objects into Image objects for icon and image properties, along with corresponding schema updates and tests. I have no further feedback to provide as there are no review comments to assess.
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.
Regenerated snapshot files reflecting the current generated code output with preprocessor blocks, static imports, and baseUrl resolution. Assisted-by: OpenCode:deepseek-v4-pro
Assisted-by: OpenCode:deepseek-v4-pro
1 participant
Background
Fedify needs a way to normalize wire-level ActivityStreams shapes before the generated vocabulary decoder applies the declared TypeScript-facing range of a property. This came up while investigating #790: ActivityStreams 2.0 allows
Object.iconandObject.imageto contain eitherImageorLink, but Fedify has exposed those properties asImage-oriented APIs since 0.4.0. RestoringImage | Linkat the TypeScript API level should wait for Fedify 3.0, so for the current major version we need a narrower compatibility path.Closes #792 and lays the groundwork for fixing #790.
What changed
@fedify/vocab-runtime
New types in packages/vocab-runtime/src/preprocessor.ts:
Json— a recursive JSON type alias for describing expanded JSON-LD valuesPropertyPreprocessorContext— the context a preprocessor receives (documentLoader,contextLoader,tracerProvider,baseUrl)PropertyPreprocessor<T>— a generic function type that receives an expanded JSON-LD property value and returnsT | undefined | Error | Promise<T | undefined | Error>@fedify/vocab-tools
Property schemas now support a
preprocessorsfield in packages/vocab-tools/src/schema.yaml and packages/vocab-tools/src/schema.ts. Each entry specifies amoduleand afunction. The module path is resolved from the generated vocabulary source file and imported dynamically at decode time.Generated decoders now run preprocessors before the normal range decoder for each expanded JSON-LD property value. The preprocessor contract:
undefined→ the normal range decoder continuesError→ decoding fails immediatelyBoth the main
fromJsonLd()decoder path (in packages/vocab-tools/src/codec.ts) and the remote-fetch#<property>_fromJsonLdhelper path (in packages/vocab-tools/src/property.ts) now emit preprocessor blocks.@fedify/vocab
New file packages/vocab/src/preprocessors.ts provides
normalizeLinkToImage, a property preprocessor that converts explicit ActivityStreamsLinkobjects intoImageobjects during decoding. It checks for expanded@typearrays containing the Link URI, parses the Link viaLink.fromJsonLd(), and mapshref,mediaType,name,width, andheightonto anImageinstance. Ifhrefis missing the preprocessor returnsundefined.The
iconandimageproperties in packages/vocab/src/object.yaml now declare this preprocessor. The public TypeScript API remainsImage-oriented.Tests
Five regression tests added to packages/vocab/src/vocab.test.ts:
All 30,428 existing tests continue to pass on Deno.
AI disclosure
This work was developed with AI assistance. All commits carry
Assisted-by: OpenCode:deepseek-v4-protrailers, and the final commit additionally carriesAssisted-by: Codex:gpt-5.5for the preprocessor expansion fix identified during code review.