Skip to content

add GitHub organization metadata filtering - #182

Merged
DavidLeuter merged 4 commits into
devfrom
feature/293-KB-gh-org-lvl-data
Sep 5, 2026
Merged

DavidLeuter merged 4 commits into
devfrom
feature/293-KB-gh-org-lvl-data

Conversation

@daniilperkin

@daniilperkin daniilperkin commented Aug 26, 2026 •

Copy link
Copy Markdown
Collaborator

Note

Merge & Review Sequence (Step 2 of 4)


📌 Summary

This PR introduces full frontend support for GitHub organization-level metadata artifacts (ORG_METADATA) in the Knowledge Base feature. It adds the dedicated Organization filter tab, organization-specific badge chip and iconography, resilient JSON metadata parsing, and an enhanced viewer drawer displaying comprehensive organization profile details without triggering unwanted redirects.


🔗 Related Issue

Closes #293


🛠️ Key Changes & Architectural Improvements

1. Types & Tolerant Parsing (src/features/knowledge-base/)

  • Extended ArtifactType: Added "ORG_METADATA" to the union type in types.ts.
  • Added metadata property: Added optional metadata?: string to the Artifact interface for backend-serialized JSON payloads.
  • Created orgMetadata.ts: Defined explicit TypeScript interfaces (OrgMetadataArtifactMetadata, OrgMetadataTeam, OrgMetadataMember) and implemented parseOrgMetadata(), a resilient parser that safely handles nulls, empty strings, and malformed JSON with graceful fallbacks and structurally invalid payloads (missing login or non-array members.

2. Knowledge Base Filtering & Navigation

  • ORGANIZATIONS Filter Tab: Added { id: "ORGANIZATIONS", label: "Organization" } to TABS in tabs.ts.
  • useKnowledgeBase Hook: Wired tab filter switch to match artifact.artifactType === "ORG_METADATA" and handle search combination and pagination reset.

3. Visuals & Artifact Viewer Drawer

  • Artifact List Iconography: Added Building2 icon from lucide-react and an "Organization" badge chip in ArtifactList.tsx.
  • Bypassed Content Fetch: Dispatched skipContentLoad: true for ORG_METADATA artifacts, preventing unwanted 302 redirects to GitHub HTML.
  • Dedicated Profile Viewer (OrgMetadataView): Renders bio, location, blog/website, email, repository counts, fully expanded teams with member badges, and direct GitHub profile links.
  • Clean Action Bar: Hides irrelevant "Summarise" and "Delete" action buttons for organization metadata.

🗂️ Modified Files Summary

Area File Summary of Changes
Types & Parser src/features/knowledge-base/types.ts Added ORG_METADATA artifact type and metadata?: string
src/features/knowledge-base/orgMetadata.ts DTO interfaces and resilient parseOrgMetadata helper
Navigation & Hook src/features/knowledge-base/tabs.ts Added ORGANIZATIONS filter tab
src/features/knowledge-base/hooks/useKnowledgeBase.ts Filter logic and page reset for organization tab
UI & Drawer src/features/knowledge-base/components/ArtifactList.tsx Added Building2 icon and Organization badge
src/features/knowledge-base/components/ArtifactViewerDrawer.tsx Profile viewer drawer (OrgMetadataView) & skip content load
Unit Tests tests/unit/features/knowledge-base/orgMetadata.test.ts Parser tests (valid, malformed, non-object, and blank input)
tests/unit/features/knowledge-base/hooks/useKnowledgeBase.test.ts Hook tab filtering, search combo, and page reset tests
tests/unit/features/knowledge-base/components/ArtifactViewerDrawer.test.tsx Drawer profile rendering and empty state fallback tests

🧪 Verification & Quality Assurance

Automated Checks

Check Tool / Command Status
Code Formatting prettier --check . ✅ Passed (0 issues)
Type Check & Build tsc -b && vite build ✅ Passed (0 errors)
Linting eslint . ✅ Passed (0 errors)
Unit Tests vitest run ✅ Passed (256 test files, 1,873 tests passed)

Manual Verification

  • Verified Organization tab appears in Knowledge Base filter bar.
  • Verified filtering displays only ORG_METADATA artifacts with Building2 icon.
  • Verified clicking an org artifact opens drawer and formats bio, teams, and members cleanly without network redirect errors.
  • Verified "Summarise" and "Delete" action buttons are hidden.

📋 Definition of Done (DoD)

  • Strict TypeScript typing enforced with no any leaks.
  • Comprehensive unit tests added covering parser edge cases, hook filters, and drawer rendering.
  • Responsive layout and accessibility standards maintained.
  • Ready for review (Frontend is complete; e2e testing pending backend org ingestion deployment).

💡 Reviewer Guidance: Please review with Claude Code and commit/push any fixes directly to this branch.

…viewer drawer

- Extend `ArtifactType` union with `ORG_METADATA` and add optional `metadata` string to `Artifact` interface.
- Add `orgMetadata.ts` containing the TypeScript types (`OrgMetadataArtifactMetadata`, `OrgMetadataTeam`, etc.) mirroring the backend DTO and a tolerant JSON parser `parseOrgMetadata`.
- Add "Organization" filter tab (`ORGANIZATIONS`) to `tabs.ts` and wire it into the `useKnowledgeBase` filter switch.
- Update `ArtifactList` with a dedicated `Building2` icon and "Organization" chip label for org artifacts.
- Update `ArtifactViewerDrawer` to skip the content fetch for `ORG_METADATA` (preventing unwanted 302 redirects to GitHub HTML), render the full org profile (description, location, blog, repo counts, teams, and members), and hide the "Summarise" action.
- Add unit tests for `parseOrgMetadata`, hook tab filtering/search combination, and drawer rendering.
@daniilperkin
daniilperkin marked this pull request as ready for review August 28, 2026 19:26
@daniilperkin

Copy link
Copy Markdown
Collaborator Author

idk when they will adress the bug, but i gues the PR can be already be reviewed

- Structural guard in parseOrgMetadata: reject payloads where login
  is not a string or members is not an array before the unchecked �s
  cast. Previously a backend payload with members: null would pass the
  plain-object check and crash the viewer when calling members.length or
  members.map().

- Add missing test case in orgMetadata.test.ts covering structurally
  invalid payloads (members: null, missing login) that previously slipped
  past the parser undetected.

- Fix KnowledgeTab import in useKnowledgeBase.ts: import from the
  canonical ../tabs module instead of indirectly through
  ../components/ArtifactFilters.

- Harden normalizeUrl: trim the input and reject strings shorter than 4
  chars before prepending https://, preventing nonsense links like
  https://N/A from being rendered when the backend stores an invalid
  blog value.

- Fix incorrect doc comment on OrgProfileRow (said 'health-check info
  row' - copy-paste error).

- Clarify OrgMetadataView doc comment: teams are always fully expanded,
  not collapsible as the previous wording implied.

Tests: 1873 passed (256 files), 0 failures. TypeScript: 0 errors.
@DavidLeuter

Copy link
Copy Markdown
Collaborator

Read the whole diff including 5e33c4d (the fixes from the last round). The structural guard in parseOrgMetadata is the right call and the drawer's skip-the-fetch path reads well. From my side this is fine to merge — one small robustness gap and a few nits, none blocking.

One thing worth fixing

parseOrgMetadata guards the top level, but OrgMetadataView also dereferences nested members.
5e33c4d rejects a payload whose top-level members isn't an array, precisely so the viewer can't call .length / .map() on a non-array. But per team the drawer does the same thing unguarded:

{team.members.length > 0 && (
  <ul …>{team.members.map((member) => …)}</ul>
)}

A team object without members (or with members: null) throws the same TypeError the top-level guard was added to prevent. Either extend the guard to the teams array, or use team.members?.length / ?? [] at the call site.

(metadata.teams itself is safe by accident — a non-array makes .length undefined, which is falsy, so the section is skipped rather than crashing.)

Nits

  • Tab label: { id: "ORGANIZATIONS", label: "Organization" } is the only singular label next to Uploads / Issues / Files / Commits, and the id is plural. "Organizations" would match the row.
  • types.ts contract: the TSDoc says metadata is "always present, default \"{}\"" but the field is declared metadata?: string. If the backend always sends it, drop the ?; if it's genuinely optional, drop "always present".
  • {@link ArtifactType.ORG_METADATA} won't resolve — ArtifactType is a string union, not an enum, so there's no .ORG_METADATA member to link to.
  • getTypeLabel doc comment reads garbled: "the org type is otherwise stored as ORG_METADATA, which the viewer users would never type themselves."
  • normalizeUrl: the trimmed.length < 4 rule does the job, but it's a proxy for "is this a URL". URL.canParse() on the prefixed value would say what you actually mean and wouldn't reject a hypothetical 3-char host. Optional.

Not approving yet per the merge sequence in the description — just flagging that I have nothing blocking here.

Reviewed with Claude Code.

…docs

- parseOrgMetadata now rejects a payload whose `teams` is present but not an
  array, or whose team entries lack a `members` array. The existing guard only
  covered the top-level `members`; the viewer maps over `team.members` for every
  team it renders, so a team without one raised the identical TypeError the
  top-level check was added to prevent.
- Rename the Knowledge Base filter tab to "Organizations" so it matches its
  siblings (Uploads / Issues / Files / Commits). The per-artifact chip stays
  singular — it labels one artifact.
- Fix the `Artifact.metadata` doc, which claimed the field is "always present"
  while typing it optional. Says why it is optional instead.
- Replace the `{@link ArtifactType.ORG_METADATA}` reference: ArtifactType is a
  union, not an enum, so the member link never resolved.
- Rewrite the garbled getTypeLabel comment.

Tests: 1875 passed (256 files). tsc -b, eslint, vite build clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@DavidLeuter

Copy link
Copy Markdown
Collaborator

Reviewed and pushed a small follow-up commit to this branch (as invited in the PR description). No behaviour change beyond the guard below.

One real bug

parseOrgMetadata rejected a payload whose top-level members isn't an array — the fix from the earlier round — but nothing checked teams[].members, and OrgMetadataView dereferences it for every team it renders:

{team.members.length > 0 && ( … team.members.map(…) )}

So a team object without a members array is the identical TypeError the top-level guard was added to prevent, one level down. The parser now rejects a teams that is present but not an array, and any team entry without a members array. Rejecting the whole payload (rather than the single team) keeps the contract callers already rely on: unusable metadata means the quiet empty state. Two tests added, plus one pinning that absent/null teams still parses.

Small stuff, same commit

  • Filter tab renamed to "Organizations" — the siblings are all plural (Uploads / Issues / Files / Commits). The per-artifact chip stays singular, it labels one artifact.
  • Artifact.metadata doc said the field is "always present" while typing it ?:. Now says why it's optional.
  • {@link ArtifactType.ORG_METADATA} doesn't resolve — ArtifactType is a union, not an enum. Replaced with a plain code span.
  • Rewrote the getTypeLabel comment, which had gotten garbled ("which the viewer users would never type themselves").

Left alone deliberately

normalizeUrl's length < 4 heuristic is arbitrary, but it's documented and it's a deliberate answer to the earlier round — not worth churning.

Verified on the branch: tsc -b, eslint, vite build clean, 1875 tests pass (256 files).

Otherwise this looks good — the tolerant parser with the structural guard, the skipContentLoad path around the 302, and the test coverage on parser edge cases are all solid.

@DavidLeuter DavidLeuter left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Approving. Re-read the branch as it stands (709f873) and, more to the point, tested what it actually looks like on top of today's dev — the branch forked at d815ae2 and dev has moved 17 commits since, so "green on the branch" and "green on merge" had stopped being the same claim.

Backmerge verified locally

Merged origin/dev into the branch in a scratch worktree. Clean, no conflicts, and nothing to reconcile by hand: those 17 commits are the chat/buddy rail, the widget picker and the dashboard edit mode, and they touch no file this PR touches. The knowledge-base feature is genuinely independent of them.

On the merge result: tsc -b, eslint . and vite build clean, 1904 of 1905 tests pass. The one failure is useChat.test.tsx > exposes stopStreaming function that can abort a stream, which

  • this PR does not touch (nothing here goes near the chatbot),
  • passes in isolation on the merge result and on plain dev, and
  • failed on a run whose suite took 547s on a loaded machine.

So: a timing-sensitive abort test flaking under parallel load, pre-existing on dev. Not this branch's problem, but worth someone pinning that test's clock at some point.

On the code

Nothing new to add over the last round. The structural guard in parseOrgMetadata now covers both levels — a top-level members that isn't an array, and any teams[] entry without one — so the viewer can't be handed a shape it dereferences blindly. Rejecting the whole payload rather than the single bad team is the right call: it keeps the one contract callers rely on (unusable metadata means the quiet empty state) instead of introducing a third, half-populated one.

skipContentLoad around the content endpoint's 302 is the neat part. The artifact has no stored bytes, so not asking for them is the honest fix rather than swallowing the error afterwards.

Merge order

Per the description this can go in whenever, independently of #183, and must land before #185. Nothing I found changes that. Given how far dev has moved, backmerge before you merge.

Reviewed with Claude Code.

Backmerge before review: the branch forked at d815ae2 and dev has moved 17
commits since. Merged clean, no conflicts.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@DavidLeuter
DavidLeuter merged commit 9b930aa into dev Sep 5, 2026
4 checks passed
@daniilperkin
daniilperkin deleted the feature/293-KB-gh-org-lvl-data branch September 15, 2026 19:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Story]: Filter Knowledge Base artifacts by GitHub org-level metadata

2 participants