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 32 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
|
Need the big picture first? Review this PR in Change Stack to see what changed before going file by file. 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
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.
Code Review
This pull request introduces property preprocessors to normalize wire-level JSON-LD property values before the generated range decoders run. Specifically, it adds the PropertyPreprocessor, PropertyPreprocessorContext, and Json types to @fedify/vocab-runtime, updates @fedify/vocab-tools to support a preprocessors field in property schemas and generate the corresponding preprocessor execution blocks in decoders, and implements a normalizeLinkToImage preprocessor in @fedify/vocab to normalize ActivityStreams Link objects in icon and image properties to Image objects. It also updates documentation configuration, gitignore rules, and adds comprehensive unit tests for the new normalization behavior. I have no feedback to provide as there are no review comments 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.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c8872aa8df
ℹ️ 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".
The URL.canParse guard on the preprocessor/getDecoder/getDecoders baseUrl expression caused a regression where relative @id values in options.baseUrl were incorrectly suppressed, breaking handler tests. Keep the safe guards on options.baseUrl fallback and root @id construction, revert the decoder-level expression to the original main-branch behavior. Assisted-by: OpenCode:deepseek-v4-pro
The stash pop conflict resolution accidentally removed tutorial and manual navigation entries, jsrRef packages, and other config sections. Restore the full config from main. 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 preprocessors to normalize wire-level JSON-LD property values before decoding. It adds PropertyPreprocessor and related types to @fedify/vocab-runtime, updates @fedify/vocab-tools to support a preprocessors field in property schemas and generate decoders that execute them, and implements a normalizeLinkToImage preprocessor in @fedify/vocab for icon and image properties. Feedback on the changes points out a potential runtime TypeError in the generated decoder code when resolving relative URLs without providing a base URL to the URL constructor.
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: a4b1a7280b
ℹ️ 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".
Resolve parent base URLs safely before decoding nested vocabulary objects, including objects whose root ID is relative and decoded with an explicit baseUrl. This keeps Link icon preprocessing and already-Image icon decoding on the same base URL path. fedify-dev#793 (comment) Assisted-by: OpenCode:deepseek-v4-pro Assisted-by: Codex:gpt-5.5
|
@codex review /gemini review |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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.
Code Review
This pull request introduces property preprocessors to the vocabulary runtime, tools, and vocab packages. It adds the PropertyPreprocessor, PropertyPreprocessorContext, and Json types to @fedify/vocab-runtime to allow normalization of wire-level JSON-LD property values before range decoding. In @fedify/vocab-tools, property schemas now support a preprocessors field, and generated decoders are updated to dynamically import and run these preprocessors. Additionally, the normalizeLinkToImage preprocessor is implemented in @fedify/vocab to normalize explicit ActivityStreams Link objects in icon and image properties to Image objects during decoding, accompanied by comprehensive unit tests. There are no review comments to address, so I have no feedback to provide.
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.
Store the baseUrl from fromJsonLd() on the instance as a protected #_baseUrl field and thread it through accessor re-parse paths. This ensures relative Link href values embedded inside icon/image properties can be resolved when the accessor is called without an explicit baseUrl and the parent object has no id. The stored URL is defensively copied to prevent callers from mutating the original URL and affecting later lazy resolution. Assisted-by: OpenCode:deepseek-v4-pro Assisted-by: Codex:gpt-5.5
The generated fromJsonLd() constructor ID expression now checks for blank node identifiers (starting with "_://") before resolving @id against the baseUrl, matching the guard already present in the property decoder baseUrl expression. Assisted-by: OpenCode:deepseek-v4-pro Assisted-by: Codex:gpt-5.5
Exclude blank-node @id from the implicit baseUrl derivation so that no baseUrl is passed without an explicit caller-supplied URL. Also extend the stored _baseUrl fallback to cached embedded re-parsing of properties that lack preprocessors, such as Document attachments. Assisted-by: OpenCode:deepseek-v4-pro Assisted-by: Codex:gpt-5.5
|
@codex review |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces property preprocessors to normalize wire-level JSON-LD property values before range decoding, specifically adding a preprocessor to normalize ActivityStreams Link objects to Image objects for icon and image properties. It also stores the baseUrl in a protected field to resolve relative URIs during lazy re-parsing and prevents resolving blank node identifiers against the base URL. Feedback on the changes includes a recommendation to guard a new URL() instantiation with URL.canParse to prevent potential runtime crashes, and suggestions to clean up redundant if-else branches in the generated property code that yield identical blocks.
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: 7eed2c609a
ℹ️ 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".
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 preprocessors to normalize wire-level JSON-LD property values before decoding, specifically normalizing ActivityStreams Link objects in icon and image properties to Image. It also updates fromJsonLd() to avoid resolving blank node identifiers against baseUrl and stores baseUrl in a protected _baseUrl field for lazy resolution. The review feedback suggests removing redundant as string type assertions, improving safety by checking values["@id"] != null instead of using the in operator, correcting documentation regarding static imports of preprocessor modules, and simplifying a null check in the preprocessor implementation.
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: Didn't find any major issues. Chef's kiss. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Remove unnecessary `as string` type assertions from generated decoder expressions where null checks already narrow the type. Use `!= null` instead of `in` for the constructor id check to make narrowing uniform. Fix schema documentation to describe static imports. Simplify the names guard in normalizeLinkToImage. fedify-dev#793 (comment) 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 preprocessors to the vocabulary generation toolchain and runtime, allowing wire-level JSON-LD property values to be normalized before decoding. It adds new types to @fedify/vocab-runtime, updates @fedify/vocab-tools to support preprocessors in schemas, and implements a normalizeLinkToImage preprocessor in @fedify/vocab to normalize ActivityStreams Link objects in icon and image properties to Image objects. Additionally, the generated base class now stores baseUrl as a protected _baseUrl field for lazy relative URI resolution, and blank node identifiers are no longer resolved against baseUrl. There are no review comments provided, so I have no feedback to offer on the review itself.
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: Didn't find any major issues. More of your lovely PRs please. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Generated decoders now reject malformed non-blank @id values instead of silently dropping them. The federation inbox parsing paths classify that validation error as permanent input failure, matching the existing JSON-LD and type validation errors. The CLI lookup tests also use runtime-specific output directories so parallel Node.js and Bun runs do not race over shared files, and docs JSR cache files generated by tests are ignored. Assisted-by: OpenCode:gpt-5.5
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.