Skip to content

fix(stats): fall back to memory filesystem scan#1425

Open
yeyitech wants to merge 2 commits intovolcengine:mainfrom
yeyitech:fix/memory-stats-fallback
Open

fix(stats): fall back to memory filesystem scan#1425
yeyitech wants to merge 2 commits intovolcengine:mainfrom
yeyitech:fix/memory-stats-fallback

Conversation

@yeyitech
Copy link
Copy Markdown
Contributor

Summary

  • fix stats category detection for viking://.../memories/profile.md
  • keep VikingDB as the primary source for /api/v1/stats/memories, but fall back to scanning the memory filesystem when no memory records are indexed
  • add regression coverage for both the profile URI case and the filesystem fallback path

Verification

  • python -m compileall openviking/storage/stats_aggregator.py tests/unit/stats/test_stats_aggregator.py tests/unit/stats/test_stats_api.py
  • manual async verification of StatsAggregator.get_memory_stats() for both profile classification and VikingFS fallback scenarios
  • pytest tests/unit/stats/test_stats_aggregator.py tests/unit/stats/test_stats_api.py -q is still blocked by the repo's known pytest-asyncio collection bug: AttributeError: 'Package' object has no attribute 'obj'

Closes #1255

@github-actions
Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis ✅

1255 - Fully compliant

Compliant requirements:

  • Fix stats category detection for profile.md
  • Add filesystem fallback when no vector-indexed memories
  • Add regression tests for both cases
⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🏅 Score: 90
🧪 PR contains tests
🔒 No security concerns identified
✅ No TODO sections
🔀 No multiple PR themes
⚡ Recommended focus areas for review

Partial Fallback Logic

The filesystem fallback is only used when records_by_uri is completely empty. If VikingDB returns some but not all memory records, the missing ones won't be supplemented from the filesystem.

if not records_by_uri:
    for record in await self._scan_memory_filesystem(ctx):
        uri = record.get("uri", "")
        if isinstance(uri, str) and uri and uri not in records_by_uri:
            records_by_uri[uri] = record
Broad Exception Swallowing

Multiple except Exception: blocks without logging may hide real errors (e.g., filesystem access issues). Add log statements to aid debugging.

except Exception:
    return
Missing Type Annotation

The viking_fs parameter in _build_filesystem_record lacks a type annotation, reducing type safety.

viking_fs,

@github-actions
Copy link
Copy Markdown

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Always scan filesystem to supplement DB records

The filesystem scan should always run to supplement VikingDB records, not only when
there are zero DB records. This ensures stats include all memories even if some are
not yet indexed.

openviking/storage/stats_aggregator.py [193-197]

-if not records_by_uri:
-    for record in await self._scan_memory_filesystem(ctx):
-        uri = record.get("uri", "")
-        if isinstance(uri, str) and uri and uri not in records_by_uri:
-            records_by_uri[uri] = record
+for record in await self._scan_memory_filesystem(ctx):
+    uri = record.get("uri", "")
+    if isinstance(uri, str) and uri and uri not in records_by_uri:
+        records_by_uri[uri] = record
Suggestion importance[1-10]: 9

__

Why: The original code only supplements with filesystem records when there are zero DB records, which contradicts the PR's stated goal of supplementing indexed records. This fix ensures all unindexed memories are included, addressing a critical logical bug.

High

@yeyitech
Copy link
Copy Markdown
Contributor Author

Updated based on the review suggestions. The stats fallback now supplements partial VikingDB results instead of only the empty-index case, and I added regression coverage for that path plus some debug logging around fallback reads.

@MaojiaSheng MaojiaSheng requested a review from chenjw April 14, 2026 06:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Backlog

Development

Successfully merging this pull request may close these issues.

Stats endpoint returns zero despite persisted memories after session commit

1 participant