Skip to content

Commit b084586

Browse files
authored
fix: correct file count and table count display in collection info (#223)
Issue 1 - File count mismatch: - CollectionCatalogInfo now accepts documentCount prop from actual API response - CollectionDrawer fetches document count from /api/documents endpoint - Uses authoritative source instead of potentially stale collection_info Issue 2 - Table count not showing: - Normalize doc_type_counts keys in _get_document_type_counts() - Convert 'structured' to 'table' when no subtype is specified - Ensures consistency with frontend expectations and derive_boolean_flags() Added unit tests for both frontend and backend changes.
1 parent 0e66303 commit b084586

File tree

6 files changed

+377
-14
lines changed

6 files changed

+377
-14
lines changed

frontend/src/components/collections/CollectionCatalogInfo.tsx

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,14 +19,19 @@ import type { Collection } from "../../types/collections";
1919

2020
interface CollectionCatalogInfoProps {
2121
collection: Collection;
22+
/** Optional override for file count - use actual document count from API for accuracy */
23+
documentCount?: number;
2224
}
2325

24-
export function CollectionCatalogInfo({ collection }: CollectionCatalogInfoProps) {
26+
export function CollectionCatalogInfo({ collection, documentCount }: CollectionCatalogInfoProps) {
2527
const info = collection.collection_info;
2628

2729
if (!info) {
2830
return null;
2931
}
32+
33+
// Use actual document count if provided, otherwise fall back to collection_info
34+
const fileCount = documentCount ?? info.number_of_files;
3035

3136
const formatDate = (dateString?: string) => {
3237
if (!dateString) return 'N/A';
@@ -124,12 +129,12 @@ export function CollectionCatalogInfo({ collection }: CollectionCatalogInfoProps
124129

125130
{/* Content Metrics */}
126131
<Flex gap="density-md" style={{ flexWrap: 'wrap' }}>
127-
{/* File Count */}
128-
{info.number_of_files !== undefined && (
132+
{/* File Count - use actual document count if available */}
133+
{fileCount !== undefined && (
129134
<Flex align="center" gap="density-xs">
130135
<FileText size={14} style={{ color: 'var(--text-color-subtle)' }} />
131136
<Text kind="body/regular/sm">
132-
{info.number_of_files} files
137+
{fileCount} files
133138
</Text>
134139
</Flex>
135140
)}

frontend/src/components/collections/CollectionDrawer.tsx

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import { useNewCollectionStore } from "../../store/useNewCollectionStore";
1818
import { useCollectionDrawerStore } from "../../store/useCollectionDrawerStore";
1919
import { useCollectionActions } from "../../hooks/useCollectionActions";
2020
import { useCollections } from "../../api/useCollectionsApi";
21+
import { useCollectionDocuments } from "../../api/useCollectionDocuments";
2122
import { DrawerActions } from "../drawer/DrawerActions";
2223
import { ConfirmationModal } from "../modals/ConfirmationModal";
2324
import { Notification, SidePanel, Stack } from "@kui/react";
@@ -44,6 +45,9 @@ export default function CollectionDrawer() {
4445
// Subscribe to collections query to sync activeCollection with fresh data
4546
const { data: collections } = useCollections();
4647

48+
// Fetch actual document count for accurate display
49+
const { data: documentsData } = useCollectionDocuments(activeCollection?.collection_name || "");
50+
4751
// Sync activeCollection with fresh data from query when collections change
4852
useEffect(() => {
4953
if (activeCollection && collections) {
@@ -117,7 +121,12 @@ export default function CollectionDrawer() {
117121
closeOnClickOutside
118122
>
119123
<Stack gap="density-md">
120-
{activeCollection && <CollectionCatalogInfo collection={activeCollection} />}
124+
{activeCollection && (
125+
<CollectionCatalogInfo
126+
collection={activeCollection}
127+
documentCount={documentsData?.total_documents}
128+
/>
129+
)}
121130
<DocumentsList />
122131
</Stack>
123132

Lines changed: 170 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,170 @@
1+
// SPDX-FileCopyrightText: Copyright (c) 2025 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
import { describe, it, expect } from 'vitest';
5+
import { render, screen } from '../../../test/utils';
6+
import { CollectionCatalogInfo } from '../CollectionCatalogInfo';
7+
import type { Collection } from '../../../types/collections';
8+
9+
describe('CollectionCatalogInfo', () => {
10+
const createMockCollection = (overrides: Partial<Collection['collection_info']> = {}): Collection => ({
11+
collection_name: 'test-collection',
12+
num_entities: 100,
13+
metadata_schema: [],
14+
collection_info: {
15+
status: 'Active',
16+
date_created: '2026-01-15T10:00:00Z',
17+
last_updated: '2026-01-15T12:00:00Z',
18+
number_of_files: 5,
19+
description: 'Test collection description',
20+
...overrides,
21+
},
22+
});
23+
24+
describe('File Count Display', () => {
25+
it('displays number_of_files from collection_info when documentCount is not provided', () => {
26+
const collection = createMockCollection({ number_of_files: 7 });
27+
render(<CollectionCatalogInfo collection={collection} />);
28+
29+
expect(screen.getByText('7 files')).toBeInTheDocument();
30+
});
31+
32+
it('displays documentCount prop when provided (overrides collection_info)', () => {
33+
const collection = createMockCollection({ number_of_files: 7 });
34+
render(<CollectionCatalogInfo collection={collection} documentCount={4} />);
35+
36+
// Should show actual document count (4) not the stale collection_info value (7)
37+
expect(screen.getByText('4 files')).toBeInTheDocument();
38+
expect(screen.queryByText('7 files')).not.toBeInTheDocument();
39+
});
40+
41+
it('displays 0 files when documentCount is 0', () => {
42+
const collection = createMockCollection({ number_of_files: 5 });
43+
render(<CollectionCatalogInfo collection={collection} documentCount={0} />);
44+
45+
expect(screen.getByText('0 files')).toBeInTheDocument();
46+
});
47+
48+
it('falls back to collection_info when documentCount is undefined', () => {
49+
const collection = createMockCollection({ number_of_files: 10 });
50+
render(<CollectionCatalogInfo collection={collection} documentCount={undefined} />);
51+
52+
expect(screen.getByText('10 files')).toBeInTheDocument();
53+
});
54+
55+
it('does not show file count when both are undefined', () => {
56+
const collection = createMockCollection({ number_of_files: undefined });
57+
render(<CollectionCatalogInfo collection={collection} documentCount={undefined} />);
58+
59+
expect(screen.queryByText(/files/)).not.toBeInTheDocument();
60+
});
61+
});
62+
63+
describe('Table Count Display', () => {
64+
it('displays table count when doc_type_counts.table is present', () => {
65+
const collection = createMockCollection({
66+
doc_type_counts: { table: 3, text: 10 },
67+
});
68+
render(<CollectionCatalogInfo collection={collection} />);
69+
70+
expect(screen.getByText('3 Tables')).toBeInTheDocument();
71+
});
72+
73+
it('does not display table count when table count is 0', () => {
74+
const collection = createMockCollection({
75+
doc_type_counts: { table: 0, text: 10 },
76+
});
77+
render(<CollectionCatalogInfo collection={collection} />);
78+
79+
expect(screen.queryByText(/Tables/)).not.toBeInTheDocument();
80+
});
81+
82+
it('does not display table count when doc_type_counts is undefined', () => {
83+
const collection = createMockCollection({
84+
doc_type_counts: undefined,
85+
});
86+
render(<CollectionCatalogInfo collection={collection} />);
87+
88+
expect(screen.queryByText(/Tables/)).not.toBeInTheDocument();
89+
});
90+
});
91+
92+
describe('Chart Count Display', () => {
93+
it('displays chart count when doc_type_counts.chart is present', () => {
94+
const collection = createMockCollection({
95+
doc_type_counts: { chart: 5, text: 10 },
96+
});
97+
render(<CollectionCatalogInfo collection={collection} />);
98+
99+
expect(screen.getByText('5 Charts')).toBeInTheDocument();
100+
});
101+
102+
it('does not display chart count when chart count is 0', () => {
103+
const collection = createMockCollection({
104+
doc_type_counts: { chart: 0, text: 10 },
105+
});
106+
render(<CollectionCatalogInfo collection={collection} />);
107+
108+
expect(screen.queryByText(/Charts/)).not.toBeInTheDocument();
109+
});
110+
});
111+
112+
describe('Image Count Display', () => {
113+
it('displays image count when doc_type_counts.image is present', () => {
114+
const collection = createMockCollection({
115+
doc_type_counts: { image: 12, text: 10 },
116+
});
117+
render(<CollectionCatalogInfo collection={collection} />);
118+
119+
expect(screen.getByText('12 Images')).toBeInTheDocument();
120+
});
121+
122+
it('does not display image count when image count is 0', () => {
123+
const collection = createMockCollection({
124+
doc_type_counts: { image: 0, text: 10 },
125+
});
126+
render(<CollectionCatalogInfo collection={collection} />);
127+
128+
expect(screen.queryByText(/Images/)).not.toBeInTheDocument();
129+
});
130+
});
131+
132+
describe('Combined Content Metrics', () => {
133+
it('displays all content types when present', () => {
134+
const collection = createMockCollection({
135+
number_of_files: 10,
136+
doc_type_counts: { table: 3, chart: 2, image: 5, text: 20 },
137+
});
138+
render(<CollectionCatalogInfo collection={collection} documentCount={10} />);
139+
140+
expect(screen.getByText('10 files')).toBeInTheDocument();
141+
expect(screen.getByText('3 Tables')).toBeInTheDocument();
142+
expect(screen.getByText('2 Charts')).toBeInTheDocument();
143+
expect(screen.getByText('5 Images')).toBeInTheDocument();
144+
});
145+
});
146+
147+
describe('Status Badge', () => {
148+
it('displays Active status with correct styling', () => {
149+
const collection = createMockCollection({ status: 'Active' });
150+
render(<CollectionCatalogInfo collection={collection} />);
151+
152+
expect(screen.getByText('Active')).toBeInTheDocument();
153+
});
154+
});
155+
156+
describe('Edge Cases', () => {
157+
it('returns null when collection_info is undefined', () => {
158+
const collection: Collection = {
159+
collection_name: 'test',
160+
num_entities: 0,
161+
metadata_schema: [],
162+
collection_info: undefined,
163+
};
164+
const { container } = render(<CollectionCatalogInfo collection={collection} />);
165+
166+
expect(container.firstChild).toBeNull();
167+
});
168+
});
169+
});
170+

frontend/src/components/collections/__tests__/CollectionDrawer.test.tsx

Lines changed: 37 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,14 @@ vi.mock('../../../store/useCollectionDrawerStore', () => ({
2424
useCollectionDrawerStore: vi.fn(() => ({
2525
activeCollection: {
2626
collection_name: 'Test Collection',
27-
metadata_schema: []
27+
metadata_schema: [],
28+
collection_info: { status: 'Active' }
2829
},
2930
closeDrawer: vi.fn(),
30-
toggleUploader: vi.fn()
31+
toggleUploader: vi.fn(),
32+
deleteError: null,
33+
showUploader: false,
34+
updateActiveCollection: vi.fn()
3135
}))
3236
}));
3337

@@ -38,14 +42,34 @@ vi.mock('../../../hooks/useCollectionActions', () => ({
3842
}))
3943
}));
4044

45+
vi.mock('../../../api/useCollectionDocuments', () => ({
46+
useCollectionDocuments: vi.fn(() => ({
47+
data: { total_documents: 5, documents: [] },
48+
isLoading: false,
49+
error: null
50+
}))
51+
}));
52+
53+
vi.mock('../../../api/useCollectionsApi', () => ({
54+
useCollections: vi.fn(() => ({
55+
data: [{
56+
collection_name: 'Test Collection',
57+
metadata_schema: [],
58+
collection_info: { status: 'Active' }
59+
}],
60+
isLoading: false,
61+
error: null
62+
}))
63+
}));
64+
4165
// Mock child components that are still used
4266
interface MockDrawerActionsProps {
4367
onDelete: () => void;
4468
onAddSource: () => void;
4569
isDeleting: boolean;
4670
}
4771

48-
vi.mock('../../drawer/DrawerActions', () => ({
72+
vi.mock('../../../components/drawer/DrawerActions', () => ({
4973
DrawerActions: ({ onDelete, onAddSource, isDeleting }: MockDrawerActionsProps) => (
5074
<div data-testid="drawer-actions">
5175
<button data-testid="delete-button" onClick={onDelete} disabled={isDeleting}>
@@ -56,14 +80,22 @@ vi.mock('../../drawer/DrawerActions', () => ({
5680
)
5781
}));
5882

59-
vi.mock('../tasks/DocumentsList', () => ({
83+
vi.mock('../../tasks/DocumentsList', () => ({
6084
DocumentsList: () => <div data-testid="documents-list">Loading documents...</div>
6185
}));
6286

63-
vi.mock('../../drawer/UploaderSection', () => ({
87+
vi.mock('../../../components/drawer/UploaderSection', () => ({
6488
UploaderSection: () => <div data-testid="uploader-section">Uploader Section</div>
6589
}));
6690

91+
vi.mock('../CollectionCatalogInfo', () => ({
92+
CollectionCatalogInfo: ({ documentCount }: { documentCount?: number }) => (
93+
<div data-testid="collection-catalog-info">
94+
{documentCount !== undefined && <span>{documentCount} files</span>}
95+
</div>
96+
)
97+
}));
98+
6799
describe('CollectionDrawer', () => {
68100
beforeEach(() => {
69101
vi.clearAllMocks();

src/nvidia_rag/ingestor_server/main.py

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2764,8 +2764,16 @@ def _get_document_type_counts(
27642764
self, results: list[list[dict[str, str | dict]]]
27652765
) -> dict[str, int]:
27662766
"""
2767-
Get document type counts from the results
2767+
Get document type counts from the results.
2768+
2769+
Note: Document types are normalized to standard keys (table, chart, image, text)
2770+
to ensure consistency with frontend expectations and derive_boolean_flags().
27682771
"""
2772+
# Mapping from nv-ingest types/subtypes to normalized keys
2773+
type_normalization = {
2774+
"structured": "table", # Structured data defaults to table
2775+
}
2776+
27692777
doc_type_counts = defaultdict(int)
27702778
total_documents = 0
27712779
total_elements = 0
@@ -2781,11 +2789,16 @@ def _get_document_type_counts(
27812789
.get("content_metadata", {})
27822790
.get("subtype", "")
27832791
)
2792+
# Use subtype if available, otherwise use document_type
27842793
if document_subtype:
2785-
document_type_subtype = document_subtype
2794+
doc_type_key = document_subtype
27862795
else:
2787-
document_type_subtype = document_type
2788-
doc_type_counts[document_type_subtype] += 1
2796+
doc_type_key = document_type
2797+
2798+
# Normalize the key to standard names (table, chart, image, text)
2799+
doc_type_key = type_normalization.get(doc_type_key, doc_type_key)
2800+
2801+
doc_type_counts[doc_type_key] += 1
27892802
if document_type == "text":
27902803
content = result_element.get("metadata", {}).get("content", "")
27912804
if isinstance(content, str):

0 commit comments

Comments
 (0)