fix(ingestion): exclude vendored/generated files from authored git LOC (#1798) - #1826
Conversation
constructorfabric#1798) Git line-count metrics classify files by extension only, so vendored deps (node_modules, site-packages, vendor/), build output (dist/, target/) and generated/minified files count as authored code — inflating per-person LOC and skewing every peer cohort that person sits in. Add a `vendored` category, checked first in both classifier call sites (the shared `git_file_category` gold macro and the inline classifier in `fct_git_file_change`). Authored line-count metrics already filter to `category = 'code'` (`code_lines_added`, `code_loc`, `clean_loc`), so they now drop vendored/generated files automatically. The total `lines_added` breakdown retains them under the `vendored` category, keeping the signal auditable rather than silently dropped. Patterns live in one place — the new `git_vendored_path_regex` macro backed by the `git_vendored_path_patterns` dbt var — so the exclusion is defined once, applied upstream, and evolvable via config without a code change. - extend the closed-taxonomy contract test with `vendored` - add a data-independent macro fixture test covering representative vendored and authored paths (holds on a fresh, empty cluster) - update schema.yml / README docs Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Anton Zelenov <antonz@constructor.tech>
|
Warning Review limit reached
Next review available in: 36 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds configurable path-based detection for vendored/generated Git files, classifies them before existing categories, excludes them from authored code-location metrics, documents the taxonomy, and adds contract coverage. ChangesVendored Git metrics
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Git file changes
participant fct_git_file_change
participant git_file_category
participant Line-count metrics
Git file changes->>fct_git_file_change: provide file paths
fct_git_file_change->>git_file_category: classify each path
git_file_category-->>fct_git_file_change: return vendored or existing category
fct_git_file_change->>Line-count metrics: provide categorized changes
Line-count metrics-->>Line-count metrics: exclude vendored from authored code-location counts
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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.
Inline comments:
In `@src/ingestion/dbt/dbt_project.yml`:
- Around line 62-66: Add the missing lockfile patterns for package-lock.json,
yarn.lock, and poetry.lock to the lockfile classification list in
dbt_project.yml, alongside the existing go.sum and Cargo.lock entries, so they
are categorized as vendored rather than matched by the fallback config rule.
In `@src/ingestion/dbt/macros/git_file_category.sql`:
- Around line 17-19: Update the git_vendored_path_regex macro to safely handle
an empty git_vendored_path_patterns override: fetch the variable with an
appropriate default and emit a regex that matches nothing when the resulting
list is empty. Preserve the existing joined-pattern regex behavior when patterns
are configured.
🪄 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: 16d3d6d7-c067-40b7-896d-67b5a98819ad
📒 Files selected for processing (7)
src/ingestion/dbt/dbt_project.ymlsrc/ingestion/dbt/macros/git_file_category.sqlsrc/ingestion/dbt/tests/gold/assert_git_file_category_vendored.sqlsrc/ingestion/dbt/tests/gold/assert_git_observations_dimension_values.sqlsrc/ingestion/silver/git/README.mdsrc/ingestion/silver/git/fct_git_file_change.sqlsrc/ingestion/silver/git/schema.yml
Address CodeRabbit review on constructorfabric#1826: - Empty `git_vendored_path_patterns` override previously made `join('|')` emit `(?i)()`, which matches every path and would flag ALL files as vendored. Guard it: emit an RE2-safe never-match `[^\s\S]` when the list is empty (RE2 has no lookahead, so `(?!)` is unavailable). - Classify the remaining common lockfiles (package-lock.json, yarn.lock, poetry.lock, Pipfile.lock, gradle.lockfile, mix.lock, flake.lock, …) as vendored too, so all machine-managed lockfiles land in one auditable bucket instead of split between `vendored` and `config`. - Extend the fixture test with lockfile cases. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Anton Zelenov <antonz@constructor.tech>
|
Addressed CodeRabbit review in 15ea04c:
|
2 participants
Closes #1798.
Problem
Git line-count metrics classify files by extension only, so machine-produced content — vendored deps (
node_modules/,site-packages/,vendor/), build output (dist/,target/), and generated/minified files — is counted as authored code. A single commit adding a vendored tree inflates a person's LOC by hundreds of thousands of lines, and because peer standing is computed against a cohort median/quartiles, one such outlier skews the benchmark for everyone in the cohort.Fix
Add a
vendoredcategory, checked first in both classifier call sites:git_file_categorygold macro (src/ingestion/dbt/macros/git_file_category.sql), consumed bygit_metric_observationsfct_git_file_change(src/ingestion/silver/git/), consumed bymtr_git_person_*Authored line-count metrics already filter to
category = 'code'(code_lines_added,code_loc,clean_loc), so they now exclude vendored/generated files automatically. The totallines_addedbreakdown retains them under thevendoredcategory, so the signal stays auditable rather than silently dropped (as the issue suggested).Patterns are defined once — the new
git_vendored_path_regexmacro backed by thegit_vendored_path_patternsdbt var (dbt_project.yml) — so the exclusion is applied upstream and evolvable via config without a code change.Acceptance criteria
code_lines_added/code_loc/clean_loc).git_vendored_path_patternsvar), not hard-coded per call site (sharedgit_vendored_path_regexmacro).assert_git_file_category_vendored.sql, data-independent so it holds on a fresh cluster).Notes
assert_git_observations_dimension_values.sql) withvendored.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation