Add license info (spdxId + name) to RepoResponse#9
Conversation
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (13)
📝 WalkthroughWalkthroughThis PR extends the repository metadata model across all layers to support three new GitHub repo fields: ChangesRepository Metadata Expansion
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 52 minutes and 23 seconds.Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/main/resources/db/migration/V15__license_info.sql (1)
21-25: 💤 Low valueConsider consolidating into a single
ALTER TABLEstatement.PostgreSQL supports multiple
ADD COLUMNclauses in oneALTER TABLE, which is atomically cleaner and slightly more idiomatic than two separate statements.✨ Suggested consolidation
-ALTER TABLE repos - ADD COLUMN IF NOT EXISTS license_spdx_id TEXT; - -ALTER TABLE repos - ADD COLUMN IF NOT EXISTS license_name TEXT; +ALTER TABLE repos + ADD COLUMN IF NOT EXISTS license_spdx_id TEXT, + ADD COLUMN IF NOT EXISTS license_name TEXT;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/resources/db/migration/V15__license_info.sql` around lines 21 - 25, The migration creates two columns on the same table using two ALTER TABLE statements; consolidate into a single ALTER TABLE on the repos table that includes both ADD COLUMN IF NOT EXISTS clauses for license_spdx_id and license_name to make the change atomic and cleaner (update V15__license_info.sql to use one ALTER TABLE repos ... ADD COLUMN IF NOT EXISTS license_spdx_id ..., ADD COLUMN IF NOT EXISTS license_name ...).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/client/license-info.md`:
- Around line 95-103: The docs overstate that license filtering is available:
update the text in docs/client/license-info.md to remove the implication that
calling /v1/search will accept arbitrary Meilisearch filter expressions and
instead note that the backend currently only accepts q, platform, sort, limit,
and offset (SearchRoutes.kt's /search handler) and that enabling filters
requires two backend changes — (1) extend the /search handler (SearchRoutes.kt)
to accept a filter parameter and safely passthrough or translate it to the
Meilisearch client, and (2) ensure the search index settings include
license_spdx_id in filterableAttributes so filter expressions work; reword the
section to mention these prerequisites and that license_spdx_id is indexed but
not yet filterable until those changes are made.
---
Nitpick comments:
In `@src/main/resources/db/migration/V15__license_info.sql`:
- Around line 21-25: The migration creates two columns on the same table using
two ALTER TABLE statements; consolidate into a single ALTER TABLE on the repos
table that includes both ADD COLUMN IF NOT EXISTS clauses for license_spdx_id
and license_name to make the change atomic and cleaner (update
V15__license_info.sql to use one ALTER TABLE repos ... ADD COLUMN IF NOT EXISTS
license_spdx_id ..., ADD COLUMN IF NOT EXISTS license_name ...).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 69707fe1-c826-4123-b60c-c035e02c0a1b
📒 Files selected for processing (15)
CLAUDE.mddocs/client/license-info.mddocs/client/open-issues-count.mdsrc/main/kotlin/zed/rainxch/githubstore/db/DatabaseFactory.ktsrc/main/kotlin/zed/rainxch/githubstore/db/MeilisearchClient.ktsrc/main/kotlin/zed/rainxch/githubstore/db/RepoRepository.ktsrc/main/kotlin/zed/rainxch/githubstore/db/Tables.ktsrc/main/kotlin/zed/rainxch/githubstore/ingest/GitHubSearchClient.ktsrc/main/kotlin/zed/rainxch/githubstore/model/RepoResponse.ktsrc/main/kotlin/zed/rainxch/githubstore/routes/InternalRoutes.ktsrc/main/kotlin/zed/rainxch/githubstore/routes/RepoRoutes.ktsrc/main/kotlin/zed/rainxch/githubstore/routes/Routing.ktsrc/main/kotlin/zed/rainxch/githubstore/routes/SearchRoutes.ktsrc/main/resources/db/migration/V14__open_issues_count.sqlsrc/main/resources/db/migration/V15__license_info.sql
| ## 6. Filter / search use cases (out of scope for this PR but FYI) | ||
|
|
||
| `licenseSpdxId` is now indexed in Meilisearch via the `license_spdx_id` field on the search document. If you want to add "filter by license" to the search screen later, it's already there — call `/v1/search` with a Meilisearch filter expression. Not implementing that here; just noting the data is available. | ||
|
|
||
| Common useful filter sets: | ||
| - "Permissive only": `MIT`, `Apache-2.0`, `BSD-2-Clause`, `BSD-3-Clause`, `MPL-2.0`, `Unlicense`, `0BSD`, `ISC`. | ||
| - "Copyleft only": `GPL-2.0`, `GPL-3.0`, `AGPL-3.0`, `LGPL-2.1`, `LGPL-3.0`. | ||
| - "Permissive or copyleft (anything but proprietary)": null exclusion + filter list. | ||
|
|
There was a problem hiding this comment.
Section 6 overstates how ready license filtering is.
"call //v1/search with a Meilisearch filter expression" implies the backend already exposes that capability. It doesn't — SearchRoutes.kt's /search handler only accepts q, platform, sort, limit, and offset; there is no path to pass arbitrary Meilisearch filter strings through to the index.
Additionally, storing license_spdx_id in documents doesn't automatically make it filterable in Meilisearch — it also needs to be added to the index's filterableAttributes settings before filter expressions work.
Suggest rewording to prevent a future developer from wiring a broken client filter without the necessary backend changes:
📝 Suggested rewording
-`licenseSpdxId` is now indexed in Meilisearch via the `license_spdx_id` field on the search document. If you want to add "filter by license" to the search screen later, it's already there — call `/v1/search` with a Meilisearch filter expression. Not implementing that here; just noting the data is available.
+`licenseSpdxId` is stored in every Meilisearch document via the `license_spdx_id` field, so the raw data is available for future filtering. To actually enable "filter by license" you would need two backend changes first:
+1. Add `license_spdx_id` to the index's `filterableAttributes` in Meilisearch settings.
+2. Expose a `license` (or similar) query parameter in `GET /v1/search` wired into `MeilisearchClient.search()`.
+Neither is in scope here; just noting the data is in the index.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ## 6. Filter / search use cases (out of scope for this PR but FYI) | |
| `licenseSpdxId` is now indexed in Meilisearch via the `license_spdx_id` field on the search document. If you want to add "filter by license" to the search screen later, it's already there — call `/v1/search` with a Meilisearch filter expression. Not implementing that here; just noting the data is available. | |
| Common useful filter sets: | |
| - "Permissive only": `MIT`, `Apache-2.0`, `BSD-2-Clause`, `BSD-3-Clause`, `MPL-2.0`, `Unlicense`, `0BSD`, `ISC`. | |
| - "Copyleft only": `GPL-2.0`, `GPL-3.0`, `AGPL-3.0`, `LGPL-2.1`, `LGPL-3.0`. | |
| - "Permissive or copyleft (anything but proprietary)": null exclusion + filter list. | |
| ## 6. Filter / search use cases (out of scope for this PR but FYI) | |
| `licenseSpdxId` is stored in every Meilisearch document via the `license_spdx_id` field, so the raw data is available for future filtering. To actually enable "filter by license" you would need two backend changes first: | |
| 1. Add `license_spdx_id` to the index's `filterableAttributes` in Meilisearch settings. | |
| 2. Expose a `license` (or similar) query parameter in `GET /v1/search` wired into `MeilisearchClient.search()`. | |
| Neither is in scope here; just noting the data is in the index. | |
| Common useful filter sets: | |
| - "Permissive only": `MIT`, `Apache-2.0`, `BSD-2-Clause`, `BSD-3-Clause`, `MPL-2.0`, `Unlicense`, `0BSD`, `ISC`. | |
| - "Copyleft only": `GPL-2.0`, `GPL-3.0`, `AGPL-3.0`, `LGPL-2.1`, `LGPL-3.0`. | |
| - "Permissive or copyleft (anything but proprietary)": null exclusion + filter list. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/client/license-info.md` around lines 95 - 103, The docs overstate that
license filtering is available: update the text in docs/client/license-info.md
to remove the implication that calling /v1/search will accept arbitrary
Meilisearch filter expressions and instead note that the backend currently only
accepts q, platform, sort, limit, and offset (SearchRoutes.kt's /search handler)
and that enabling filters requires two backend changes — (1) extend the /search
handler (SearchRoutes.kt) to accept a filter parameter and safely passthrough or
translate it to the Meilisearch client, and (2) ensure the search index settings
include license_spdx_id in filterableAttributes so filter expressions work;
reword the section to mention these prerequisites and that license_spdx_id is
indexed but not yet filterable until those changes are made.
8904240 to
6d73c63
Compare
Summary
Surface a repo's license on the details screen + everywhere else
RepoResponseis returned. Same pattern asopenIssuesCount(#8) -- rides on the existing repo response shape, no new endpoint, no extra fetch.Two new fields:
licenseSpdxId: String?-- short tag ("MIT","GPL-3.0","Apache-2.0")licenseName: String?-- full name ("MIT License")Both nullable. GitHub returns
license: nullfor repos without aLICENSEfile or unrecognised licenses.Changes
Per
CLAUDE.md's RepoResponse-fan-out checklist:db/migration/V15__license_info.sql-- addslicense_spdx_id TEXT+license_name TEXT, both nullable. Idempotent.db/DatabaseFactory.kt-- registers V15.db/Tables.kt-- two new Exposed columns onRepos.model/RepoResponse.kt-- two new fields withnulldefaults.db/RepoRepository.kt-- maps both columns.db/MeilisearchClient.kt-- adds the fields toMeiliRepoHit.ingest/GitHubSearchClient.kt-- newGitHubLicenseDTO (only persistsspdx_id+name); upsert + sync write both columns;RepoWithRelease.toRepoResponsecarries them.routes/RepoRoutes.kt--toMetadataOnlyResponse()carries them.routes/SearchRoutes.kt--MeiliRepoHit.toRepoResponse()carries them.Existing curated rows have
nulllicense fields until any write path fires (search ingest, refresh button, hourly worker, daily Python fetcher).Why drop GitHub's
keyandurlkeyisspdx_id.lowercased()-- redundant.urlpoints at GitHub's API, not a user-visible URL. Client links tohttps://github.com/{owner}/{name}/blob/HEAD/LICENSEwhich is always correct.Client integration
docs/client/license-info.md-- standalone guide for the client agent. Covers display recommendations (chip with tooltip, no color-coding, hide cleanly when null), tap behaviour (open GitHub LICENSE file), back-compat, explicit non-goals.Test plan
./gradlew test-- all suites green.POST /v1/repo/sindresorhus/refined-github/refresh-- expectlicenseSpdxId+licenseNamepopulated.nullfor both fields.Summary by CodeRabbit
Release Notes
New Features
Documentation