Skip to content

Conversation

@maximeallanic
Copy link
Member

@maximeallanic maximeallanic commented Jun 5, 2025

PR Type

Enhancement


Description

  • Import FullMetadata type

  • Initialize URL-to-promise metadata cache

  • Implement caching in getMetadata

  • Return cached metadata on repeated calls


Changes walkthrough 📝

Relevant files
Enhancement
storage.ts
Introduce metadata caching in storage                                       

stores/storage.ts

  • Imported FullMetadata from storage API
  • Added cachedMetadata mapping URLs to promises
  • Updated getMetadata to store metadata promise
  • Check and return cached promise before fetching
  • +8/-2     

    Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
  • @github-actions github-actions bot enabled auto-merge (squash) June 5, 2025 12:07
    @github-actions
    Copy link

    github-actions bot commented Jun 5, 2025

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Variable typo

    In getMetadata, the cache lookup uses cachedMetadata[path] but the function parameter is named url. This will cause a reference error and prevent caching from working.

     if (cachedMetadata[path] !== undefined) {
        return cachedMetadata[path];
    }
    Unused variable

    The cached object is declared but never used. You should remove it or implement its intended caching logic to avoid dead code.

    const cached = {}, cachedMetadata: {[key: string]: Promise<FullMetadata>} = {};

    @github-actions
    Copy link

    github-actions bot commented Jun 5, 2025

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Use url instead of undefined path

    The path variable is undefined in this context and should be replaced with the
    function parameter url to correctly key the cache. This will fix the cache lookup
    logic and prevent runtime errors.

    stores/storage.ts [78-80]

    -if (cachedMetadata[path] !== undefined) {
    -   return cachedMetadata[path];
    +if (cachedMetadata[url] !== undefined) {
    +   return cachedMetadata[url];
     }
    Suggestion importance[1-10]: 9

    __

    Why: The cache lookup mistakenly uses path, which is undefined in this scope, causing a runtime error; replacing it with url is a critical bug fix.

    High

    }

    async function getMetadata(url: string) {
    if (cachedMetadata[path] !== undefined) {
    Copy link

    Choose a reason for hiding this comment

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

    The path variable in the getMetadata function's cache check is undefined; it should be url to match the parameter name.

    BETA Uses AI. Verify results. Give Feedback

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Projects

    None yet

    Development

    Successfully merging this pull request may close these issues.

    2 participants