Skip to content

docs(agents): path prefix-root conventions + cross-project cache-key pitfalls#961

Merged
zackees merged 1 commit into
mainfrom
fix/952-core-cache-key
Jul 5, 2026
Merged

docs(agents): path prefix-root conventions + cross-project cache-key pitfalls#961
zackees merged 1 commit into
mainfrom
fix/952-core-cache-key

Conversation

@zackees

@zackees zackees commented Jul 5, 2026

Copy link
Copy Markdown
Member

Summary

Adds agents/docs/path-conventions.md — durable guidance that came out of the #942/#952 profiling work. Documents:

  • The path prefix-root families and the factory functions that pick them: ~/.fbuild/{dev|prod}/cache/… via get_cache_root() / Cache::new(project_dir).*_dir() (packages/ toolchains/ platforms/ libraries/ core/), the global root via get_fbuild_root(), and per-project <project>/.fbuild/build/{env}/{profile}/ via BuildLayout.
  • Why the {dev|prod} segment must come from is_dev_mode() (never hardcoded) and why global caches ignore project_dir.
  • The cross-project cache-key pitfall: comparing an un-normalized path against a canonicalized project_dir with raw Path::starts_with silently misses, so a cache key encodes the project directory and never hits across project dirs. Normalize both sides through fbuild_core::path::normalize_for_key; keep cross-project keys project-independent; keep zccache-visible compile keys workspace-relative.
  • A pre-commit checklist for path/cache-key changes.

Wired into the root CLAUDE.md routing table and the agents/docs/README.md index.

Docs-only; no code changes.

Part of #942 (the perf sub-issues #952/#953 remain open — see the #952 root-cause comment).

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Documentation
    • Added guidance on path and cache directory conventions, including where build and cache files should live.
    • Clarified how cache keys are expected to behave across projects and environments.
    • Expanded documentation with rules for comparing paths consistently and avoiding path-related cache issues.

…pitfalls

Adds agents/docs/path-conventions.md documenting the two path prefix-root
families (~/.fbuild/{dev|prod}/cache via get_cache_root/Cache factories vs
<project>/.fbuild/build/{env}/{profile} via BuildLayout), the factory
functions that pick them, and the cross-project cache-key pitfall found
while profiling #942/#952: comparing an un-normalized path against a
canonicalized project dir silently defeats a cache key. Includes a
pre-commit checklist and wires the doc into the CLAUDE.md routing table
and agents/docs README.

Part of #942

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 5, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 0a0cdd5d-51ec-41bd-bb08-822a711e96bb

📥 Commits

Reviewing files that changed from the base of the PR and between 81543e4 and 67265a0.

📒 Files selected for processing (3)
  • CLAUDE.md
  • agents/docs/README.md
  • agents/docs/path-conventions.md

📝 Walkthrough

Walkthrough

This PR adds a new documentation file, agents/docs/path-conventions.md, describing path/cache-key conventions for the fbuild system, and adds cross-references to it from CLAUDE.md and agents/docs/README.md.

Changes

Path conventions documentation

Layer / File(s) Summary
New path-conventions guidance document
agents/docs/path-conventions.md
New doc detailing global prefix roots, dev/prod cache root selection, project-independent cache-key rules, cross-project key isolation, path normalization requirements, workspace-relative zccache compile keys, and a commit checklist referencing fbuild crates.
Routing links to new doc
CLAUDE.md, agents/docs/README.md
Adds a routing table entry and a file-list bullet pointing to path-conventions.md, with brief guidance on cache/build path structure and cache-key mismatch causes.

Estimated code review effort: 1 (Trivial) | ~3 minutes

Related PRs: References #695 and #952.

Suggested labels: documentation

Suggested reviewers: None identified from the provided information.

🐰 A rabbit hops through docs anew,
Cache roots and paths now clear in view,
Dev or prod, the keys align,
No stale hits or paths misassigned,
Conventions written, checklist too!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the new docs on path prefix-root conventions and cache-key pitfalls.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/952-core-cache-key

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@zackees
zackees merged commit 6306da3 into main Jul 5, 2026
12 of 14 checks passed
@fastled-project-sync fastled-project-sync Bot moved this to Triage in FastLED Tracker Jul 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Triage

Development

Successfully merging this pull request may close these issues.

1 participant