fix: skills apis backend response bloat clearing + cleanup orphans grace period change

[danpiths]

Summary

This PR decouples version history loading from the GetSkill endpoint, extends
the orphan file cleanup grace period from 30 minutes to 24 hours, and populates
HighestVersion and FileCount fields directly on the skill struct during
retrieval.

Changes

• GetSkill no longer eagerly preloads the full version list. Instead, it
  fetches only the most recently created version via latestCreatedSkillVersion
  and assigns it to skill.HighestVersion for bump validation. Full version
  history is now exclusively loaded through ListSkillVersions.
• populateSkillFiles now also sets skill.FileCount from the length of the
  loaded files slice.
• The orphan blob and upload object cleanup grace period has been increased from
  30 minutes to 24 hours, extracted into a named constant
  SkillOrphanCleanupGracePeriod shared between the configstore and HTTP
  handler layers.
• Tests updated to backdate orphan blobs and objects by 25 hours instead of 1
  hour to correctly exceed the new grace period.
• The SkillVersionSummary type comment updated to reflect that it is only
  returned in version list responses, not skill detail responses.

Type of change

• Bug fix
• Feature
• Refactor
• Documentation
• Chore/CI

Affected areas

• Core (Go)
• Transports (HTTP)
• Providers/Integrations
• Plugins
• UI (React)
• Docs

How to test

go test ./framework/configstore/...
go test ./transports/bifrost-http/handlers/...

Verify that:

• GetSkill returns the correct HighestVersion without including a full
  version list in the response.
• FileCount is populated correctly on the returned skill.
• Orphan cleanup does not reap blobs or objects created within the last 24 hours
  when force is false.
• Orphan cleanup correctly removes blobs and objects older than 24 hours.

Breaking changes

• Yes
• No

Related issues

Security considerations

None.

Checklist

• I read docs/contributing/README.md and followed the guidelines
• I added/updated tests where appropriate
• I updated documentation where needed
• I verified builds succeed (Go and UI)
• I verified the CI pipeline passes locally if applicable

[danpiths]

• fix: skills apis backend response bloat clearing + cleanup orphans grace period change #4396 👈 (View in Graphite)
• dev

This stack of pull requests is managed by Graphite. Learn more about stacking.

[coderabbitai]

📝 Walkthrough

Summary by CodeRabbit

• Performance Improvements

  • Skills load faster with streamlined version history retrieval, eliminating unnecessary preloading of full version data

• Improvements

  • Skill file count now accurately reflects the actual number of loaded files
  • Extended orphan file cleanup grace period to 24 hours, providing better protection for recent uploads and improved data recovery windows

Walkthrough

Introduces a SkillOrphanCleanupGracePeriod constant (24 hours) and applies it to both the DB-fallback blob cleanup in configstore and the upload-object cleanup in the HTTP handler, replacing hardcoded 30-minute offsets. Tests are updated to backdate orphans by 25 hours. GetSkill is optimized to avoid full version-history preloads, and populateSkillFiles now sets FileCount.

Changes

Skill Orphan Cleanup Grace Period and GetSkill Optimization

Layer / File(s)	Summary
GetSkill: remove version preload, set FileCount framework/configstore/skills.go	GetSkill stops preloading the full Versions relation and instead fetches only the latest created version string to set HighestVersion. populateSkillFiles now assigns skill.FileCount from the count of loaded files.
SkillOrphanCleanupGracePeriod constant and DB blob cleanup framework/configstore/skills.go	Adds the exported SkillOrphanCleanupGracePeriod = 24 * time.Hour constant and updates CleanupOrphanSkillFileBlobs to use it instead of a hardcoded 30-minute cutoff when force is false.
HTTP handler cleanup updated to use constant transports/bifrost-http/handlers/skills.go	CleanupOrphanSkillFiles now computes the deletion cutoff using configstore.SkillOrphanCleanupGracePeriod instead of a hardcoded 30-minute offset; comments updated to reflect the 24-hour window.
Cleanup tests: 25h backdating alignment transports/bifrost-http/handlers/skills_cleanup_test.go	Both orphan-cleanup test cases now backdate resources by 25 hours (-25*time.Hour) instead of 1 hour, matching the new 24-hour grace period.
SkillVersionSummary comment ui/lib/types/skills.ts	Adds a clarifying comment that SkillVersionSummary is the lean version summary returned by version list and skill detail responses.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• maximhq/bifrost#4229: Directly related — touches GetSkill/HighestVersion computation and orphaned skill file blob cleanup logic that this PR extends.
• maximhq/bifrost#4231: Introduced CleanupOrphanSkillFiles with the original 30-minute grace period that this PR replaces with the new 24-hour constant.

Suggested reviewers

• akshaydeo
• Pratham-Mishra04

Poem

🐇 Thirty minutes felt too hasty, said the rabbit with a sniff,
"Let orphaned blobs sit longer, give them twenty-four to drift!"
Now SkillOrphanCleanupGracePeriod keeps the constants neat,
And GetSkill skips the version load — a trim, efficient feat.
Hop hop, the tests agree: twenty-five hours is the key! 🌙

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the two main changes: decoupling version history loading (response bloat clearing) and extending the orphan cleanup grace period, matching the PR's core objectives.
Description check	✅ Passed	The description is comprehensive and follows the template structure, covering summary, changes, type of change, affected areas, testing instructions, breaking changes, and checklist items with all required sections properly completed.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch 06-15-fix_skills_apis_backend_response_bloat_clearing_cleanup_orphans_grace_period_change

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.12.2)

level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies"

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[greptile-apps]

Confidence Score: 4/5

Safe to merge; changes are well-scoped and test coverage for the cleanup behaviour is updated correctly.

The core logic changes are correct and consistent with the existing GetSkillLean pattern. GetSkill and GetSkillLean are now functionally identical — a cleanup opportunity — and no new test directly asserts GetSkill's HighestVersion output after the refactor, though the path is implicitly covered. Neither finding affects runtime correctness today.

framework/configstore/skills.go — GetSkill and GetSkillLean are now equivalent; worth consolidating to prevent silent divergence.

Important Files Changed

Filename	Overview
framework/configstore/skills.go	Removes version preload from GetSkill, adds latestCreatedSkillVersion and FileCount population; introduces SkillOrphanCleanupGracePeriod constant (30 min to 24 h). GetSkill and GetSkillLean are now functionally identical; no new test for GetSkill's HighestVersion behavior.
transports/bifrost-http/handlers/skills.go	Updates object-store orphan cutoff to use the shared SkillOrphanCleanupGracePeriod constant from configstore; comment-only clarification for the grace-period description. No logic issues.
transports/bifrost-http/handlers/skills_cleanup_test.go	Backdates orphan blobs and objects from -1 h to -25 h to correctly exceed the new 24-hour grace period; tests remain coherent and accurate.
ui/lib/types/skills.ts	Comment-only update: SkillVersionSummary doc updated to reflect it is now returned only in version-list responses, not skill-detail responses.

Comments Outside Diff (1)

1. framework/configstore/skills.go, line 655-701 (link)

   GetSkill and GetSkillLean now have identical implementations

   After this change, both functions perform the same three operations: First on skills.id, populateSkillFiles, and latestCreatedSkillVersion to set HighestVersion. The "lean" distinction was meaningful when GetSkill preloaded the full Versions slice and GetSkillLean did not — that distinction is now gone. Any future maintainer who reaches for GetSkillLean because they want to avoid loading versions will be surprised to find GetSkill already behaves the same way, and vice-versa. Consider consolidating into a single method (or renaming GetSkillLean to GetSkill with deprecation of the duplicate) to avoid silent drift if the implementations diverge again.

   Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

_{Reviews (1): Last reviewed commit: "fix: skills apis backend response bloat ..." | Re-trigger Greptile}

[coderabbitai]

🧹 Nitpick comments (1)

ui/lib/types/skills.ts (1)

47-51: 💤 Low value

Comment wording: suggest explicit exclusivity clause to match PR objectives.

The comment "Lean version summary returned in version list responses" is accurate and helpful, but the PR objectives state the clarification should emphasize it is returned "only in version list responses, not in skill detail responses." While the TypeScript interfaces already enforce this (GetSkillResponse uses Skill without a versions field), making the comment explicit would align it with the stated intent.

Optional improvement: "Lean version summary returned only in version list responses, not in skill detail responses."

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@ui/lib/types/skills.ts` around lines 47 - 51, Update the comment for the
SkillVersionSummary type definition to explicitly clarify that it is returned
only in version list responses, not in skill detail responses. Change the
current comment from "Lean version summary returned in version list responses."
to "Lean version summary returned only in version list responses, not in skill
detail responses." to align with the PR objectives and make the distinction
between usage contexts clear.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@ui/lib/types/skills.ts`:
- Around line 47-51: Update the comment for the SkillVersionSummary type
definition to explicitly clarify that it is returned only in version list
responses, not in skill detail responses. Change the current comment from "Lean
version summary returned in version list responses." to "Lean version summary
returned only in version list responses, not in skill detail responses." to
align with the PR objectives and make the distinction between usage contexts
clear.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: a1dc94b2-13f4-4206-b0b3-717f4292e688

📥 Commits

Reviewing files that changed from the base of the PR and between 9a4bc66 and 0e73446.

📒 Files selected for processing (4)

• framework/configstore/skills.go
• transports/bifrost-http/handlers/skills.go
• transports/bifrost-http/handlers/skills_cleanup_test.go
• ui/lib/types/skills.ts

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[akshaydeo]

Merge activity

• Jun 15, 10:58 AM UTC: A user started a stack merge that includes this pull request via Graphite.
• Jun 15, 10:59 AM UTC: @akshaydeo merged this pull request with Graphite.