Skip to content

Fix/add resource cover#1338

Merged
MaojiaSheng merged 3 commits intomainfrom
fix/add_resource_cover
Apr 9, 2026
Merged

Fix/add resource cover#1338
MaojiaSheng merged 3 commits intomainfrom
fix/add_resource_cover

Conversation

@myysy
Copy link
Copy Markdown
Collaborator

@myysy myysy commented Apr 9, 2026

Description

Improve add-resource behavior when importing into viking://resources, and preserve stable resource naming for uploaded files and zip archives. This branch fixes root-target imports so they create child resources under the resources root, keeps single-file imports using the original filename stem, and ensures single-root zip archives use the extracted top-level directory name instead of the uploaded .zip filename.

Related Issue

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactoring (no functional changes)
  • Performance improvement
  • Test update

Changes Made

  • Treat --to viking://resources and viking://resources/ as imports under the resources root instead of as exact destination URIs, while preserving child URIs for downstream dedup and incremental update flows.
  • Send source_name for uploaded files from both Rust and Python clients so single-file imports keep the original filename stem as the resource directory name.
  • Update zip processing so archives with a single extracted top-level directory use that real directory name instead of inheriting the uploaded .zip filename.
  • Split summarizer enqueue behavior for imports targeting the resources root so each top-level imported child is enqueued with its own viking://resources/<child> target URI.
  • Add regression coverage for tree building, API behavior, summarizer root splitting, uploaded file naming, and single-root zip naming.

Testing

  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I have tested this on the following platforms:
    • Linux
    • macOS
    • Windows
      Test notes:
  • Added/updated tests in:
    • tests/server/test_api_resources.py
    • tests/misc/test_tree_builder_dedup.py
    • tests/misc/test_media_processor_zip_root.py
    • tests/unit/test_summarizer_resources_root_split.py
  • Attempted to run:
    • pytest tests/misc/test_media_processor_zip_root.py
  • Local test execution is currently blocked by a missing dependency in the environment:
    • ModuleNotFoundError: No module named 'litellm'

Checklist

  • My code follows the project's coding style
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • Any dependent changes have been merged and published

Screenshots (if applicable)

N/A

Additional Notes

Branch diff summary against main:

  • 2 commits ahead of main
  • Changed files:
    • crates/ov_cli/src/client.rs
    • openviking/parse/parsers/markdown.py
    • openviking/parse/tree_builder.py
    • openviking/utils/media_processor.py
    • openviking/utils/summarizer.py
    • openviking_cli/client/http.py
    • tests/misc/test_media_processor_zip_root.py
    • tests/misc/test_tree_builder_dedup.py
    • tests/server/test_api_resources.py
    • tests/unit/test_summarizer_resources_root_split.py
      Diff stat against main:
  • 10 files changed
  • 568 insertions
  • 33 deletions

myysy added 2 commits April 9, 2026 20:18
- Add source_name to file upload requests for preserving original filenames
- Handle single-directory zip files by using their root directory directly
- Support viking://resources as parent directory for imports
- Split summarization for resources root imports into individual child items
- Add tests for new resource import behaviors
@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 9, 2026

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🏅 Score: 92
🧪 PR contains tests
🔒 No security concerns identified
✅ No TODO sections
🔀 No multiple PR themes
⚡ Recommended focus areas for review

Inconsistent License Header

The new test file uses Apache-2.0 license while all other test files in the repo use AGPL-3.0, which is inconsistent with the project's licensing convention.

# SPDX-License-Identifier: Apache-2.0
Redundant Code

The kwargs.setdefault("source_name", source_name) call is redundant when source_name is truthy because source_name was just retrieved from kwargs.get("source_name"), so the key is already present in kwargs.

kwargs.setdefault("source_name", source_name)

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 9, 2026

PR Code Suggestions ✨

No code suggestions found for the PR.

Improve readability by applying consistent line breaks in test file patches and removing trailing whitespace
@MaojiaSheng MaojiaSheng merged commit a18d4b9 into main Apr 9, 2026
10 of 11 checks passed
@MaojiaSheng MaojiaSheng deleted the fix/add_resource_cover branch April 9, 2026 12:31
@github-project-automation github-project-automation bot moved this from Backlog to Done in OpenViking project Apr 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants