Repository navigation
All time filter - activity counts are not adding up #413
Description
Activity
Coding Plan
Summary
The defect is a pipeline-layer wiring error. The base ("All time")
role_coveragetable computes its*_recentcount columns andstatusfrom a fixed 90-dayROLE_ACTIVE_DAYSwindow instead of from all-time data.Fix the single base-table call in
pipelines/role_coverage.py::main()to use all-time profiles andactive_within_days=None. This fits the layer rules, because the pipeline owns the window decision and theanalysisfunction already supports the all-time case.The fix corrects all four affected Governance sections (
repo,repoactivity,understaffed,loadshare) at once, because they all derive from the same base table.Add a regression test for the "All time ≥ 1 year" superset invariant and confirm the output contract and column schema are unchanged.
Design Choices
Design Choice 1: Scope of the all-time fix: status column and count columns
Options Considered:
- Make the base table fully all-time. Pass the all-time profile set as
recent_profilesand passactive_within_days=Nonefor the base call. The*_recentcounts become all-time, and a role holder is "active" if they have any recorded activity. This matches the module docstrings ("Status and days-since-active reflect all-time recency"), the existinggonedarkbase file (which already usesdark_after_days=None), and the prior#286fix pattern. - Make only the count columns all-time, but keep
statuson the 90-dayROLE_ACTIVE_DAYSthreshold. Pass the all-time profile set asrecent_profilesbut keepactive_within_days=ROLE_ACTIVE_DAYS. This fixes the reported count mismatch but leaves the basestatus/active_maintainerson a 90-day window, which still differs from the period tabs' windowed status.
Chosen Option: 1
Rationale: Option 1 removes the whole base-vs-period inconsistency in one place. It agrees with the existing docstrings and the
gonedark/#286precedent. It makes the "All time" tab a true superset for every affected column, includingstatusandactive_maintainers.💡 User Tips
Regenerate the plan with different choices with
@coderabbitai <feedback>.Implementation Steps
Phase 1: Correct the all-time base table in the role_coverage pipeline
This phase makes the base file represent true all-time data for its count and status columns. It keeps the 90-day
ROLE_ACTIVE_DAYSwindow only where it is a genuine, tab-independent concept.Task 1: Build the base coverage table from all-time profiles
Make the base
role_coverage_all.csvcompute its windowed columns from the full record history.- Update the base-table call in
pipelines/role_coverage.py::main()(thecombined = _build_role_coverage(...)call). - Pass the all-time per-repo profile set (
all_time_by_repo) as therecent_profilesargument, so*_recentcounts equal the all-time counts for the base file. - Pass
active_within_days=Nonefor the base call, sostatusmarks a role holder active if they have any recorded activity. - Keep the separate 90-day
recent_by_repovalue produced by_build_profile_sets()unchanged. It is still needed for the co-membership network charts (_write_role_networks→build_active_membership), which are not driven by the period tabs. - Do not change the
for period in ACTIVITY_PERIODS:loop. The period variants (_7d,_30d,_365d) are already correct.
Task 2: Confirm derived base summaries inherit the all-time fix
Make sure the three dependent Governance sections become all-time through the corrected base table.
- Verify in
pipelines/role_coverage.pythat_write_repo_summaries(combined, ...)(theperiod=Nonecall) derivesrepo_activity_overview.csv,maintainer_coverage_risk.csv, andreview_load_share.csvfrom the corrected basecombinedtable. - No separate change is expected here if these summaries read from
combined. If any of them recompute their own 90-day window instead of readingcombined, apply the same all-time correction (all-time counts,active_within_days=None) to that base call.
Task 3: Align docstrings and comments with the corrected behavior
Remove the mismatch between the documented invariant and the code.
- Update the docstrings/comments in
pipelines/role_coverage.py(_build_profile_sets,_build_role_coverage, and the base-table section ofmain()) to state clearly that the base file's*_recentcounts andstatusare all-time, and that the 90-dayROLE_ACTIVE_DAYSwindow now feeds only the network charts. - Do not change the meaning of
ROLE_ACTIVE_DAYSinconfig/analysis.py. It remains the network-chart recency window and the per-reporole_coverage.csv/role_promotion_candidates.csvthreshold.
🤖 Prompt for AI agents
Implement the fix for the all-time base table in `src/hiero_analytics/pipelines/role_coverage.py`. The goal: the base ("All time") `role_coverage` table must compute its `*_recent` count columns and `status` from all-time data, not from a fixed 90-day window. - In `main()`, find the base-table call `combined = _build_role_coverage(...)`. Change its `recent_profiles` argument to use the all-time per-repo profile set (`all_time_by_repo`) instead of the 90-day `recent_by_repo`. Change `active_within_days` to `None` for this base call so `status` marks a role holder active based on any recorded activity, not a 90-day cutoff. - Do not modify `_build_profile_sets()`'s 90-day `recent_by_repo` value; it must remain available and unchanged, because it still feeds the co-membership network charts (`_write_role_networks` → `build_active_membership`). - Do not modify the `for period in ACTIVITY_PERIODS:` loop that rebuilds the `_7d`, `_30d`, and `_365d` variants; these are already correct. - Verify that `_write_repo_summaries(combined, ..., period=None)` derives `repo_activity_overview.csv`, `maintainer_coverage_risk.csv`, and `review_load_share.csv` from the corrected `combined` table. If any of these three summaries independently recompute a 90-day window instead of reading from `combined`, apply the same all-time correction (all-time counts, `active_within_days=None`) to that computation. - Update the docstrings and comments in `_build_profile_sets`, `_build_role_coverage`, and the base-table section of `main()` to state that the base file's `*_recent` counts and `status` are all-time, and that the 90-day `ROLE_ACTIVE_DAYS` window now applies only to network charts. - Do not change the meaning or value of `ROLE_ACTIVE_DAYS` in `config/analysis.py`. It must keep governing the network-chart recency window and the per-repo `role_coverage.csv`/`role_promotion_candidates.csv` threshold.Phase 2: Regression tests and output-contract verification
This phase pins the corrected invariant with tests and confirms the output contract is unchanged.
Task 1: Add a pipeline regression test for the all-time superset invariant
Prove that the base file counts are never smaller than the 365-day counts.
- Extend
tests/pipelines/test_role_coverage.py. - Build a synthetic activity fixture that includes review/PR events older than 90 days but inside 365 days for at least one role holder in one repo.
- Run
main()and assert, per role holder and repo, that each base*_recentvalue inrole_coverage_all.csvis greater than or equal to the matching value inrole_coverage_all_365d.csv. - Assert that the base
*_recentcounts equal the true all-time counts for that fixture (the events older than 90 days are included). - Assert that a role holder whose only activity is older than 90 days is
active(not quiet) in the base file, to lock the all-timestatusdecision from Task 1.1.
Task 2: Confirm the output contract and column schema are unchanged
Confirm no column is renamed or added, so the contract test still passes without edits.
- Do not rename or add columns in
dashboard_spec/governance.pyfor therepo,repoactivity,understaffed, orloadsharesections. The fix changes only the values in existing columns. - Run
tests/contracts/test_output_contract.pyto confirm the spec↔producer contract still holds. - If a later design choice requires a column rename or a new column, update the matching
SECTION_SPECSentry indashboard_spec/governance.py. The contract test derives its expectations from the spec at run time, so it needs no direct edit.
🤖 Prompt for AI agents
Add regression tests and verify the output contract for the all-time fix made in Phase 1. - Extend `tests/pipelines/test_role_coverage.py` with a new test. Build a synthetic activity fixture that includes review/PR events older than 90 days but within 365 days, for at least one role holder in one repo. - Run `main()` against this fixture, then load `role_coverage_all.csv` and `role_coverage_all_365d.csv`. - Assert that, for each role holder and repo, every base `*_recent` value (`reviews_recent`, `prs_recent`, `merges_recent`, `issues_recent`, `labels_recent`) in `role_coverage_all.csv` is greater than or equal to the corresponding value in `role_coverage_all_365d.csv`. - Assert that the base `*_recent` counts equal the true all-time counts, including the events older than 90 days. - Assert that a role holder whose only activity is older than 90 days has `status` equal to `active` (not quiet) in the base file. - Do not rename or add any column in `dashboard_spec/governance.py` for the `repo`, `repoactivity`, `understaffed`, or `loadshare` sections; the fix changes only column values, not the schema. - Run `tests/contracts/test_output_contract.py` and confirm it still passes, since it derives its expectations from `SECTION_SPECS` at run time. - Only if a later design decision requires a column rename or addition, update the matching `SECTION_SPECS` entry in `dashboard_spec/governance.py`; otherwise leave it unchanged.Research
Ticket Summary
The "All time" filter shows smaller activity counts than the "1 year" filter in several Governance tab widgets. The reported example is
exploreriiireviews in the python SDK. The task is to make the "All time" filter count truly all-time data, so it is always a superset of any period window.Codebase Summary
The project follows a strict layered architecture (
docs/architecture.md):config/domaindefine shared vocabulary,data_sourcesfetch and persist GitHub data,analysisperforms pure DataFrame transforms,export/dashboard_specrender output and the JSON API, andpipelinesorchestrate fetch → analysis → output.Time filtering uses
domain/periods.py.ACTIVITY_PERIODScontains onlyWeek(7d),1 month(30d), and1 year(365d). "All time" is not a period. It is the base table that each pipeline writes before it loops overACTIVITY_PERIODS. The web app (web/src/components/SectionTable.tsx,PeriodTabs.tsx) selects the baserowsfor the "All time" tab (period === null) andperiods[key]for a period tab, and it always uses the same columns.The Governance "repo" widget is produced by
pipelines/role_coverage.pyandanalysis/role_coverage.py. It is declared indashboard_spec/governance.py.Root Cause
pipelines/role_coverage.pybuilds the base filerole_coverage_all.csvincorrectly for its windowed columns._build_profile_sets()buildsrecent_by_repofrom records filtered to a fixed 90-day window (ROLE_ACTIVE_DAYS,config/analysis.py).main()calls_build_role_coverage(...)for the base table with that 90-dayrecent_by_repoand letsactive_within_daysdefault toROLE_ACTIVE_DAYS(90 days).- The base file's
*_recentcolumns (reviews_recent,prs_recent,merges_recent,issues_recent,labels_recent) and itsstatustherefore reflect the last 90 days, not all time. - The period loop rebuilds each variant (
_7d,_30d,_365d) with the correct window, sorole_coverage_all_365d.csvuses a 365-day window.
The Governance "repo" widget maps
reviews_recentto the displayed "reviews" column. The "All time" tab reads the base file (90-day window). The "1 year" tab reads the 365-day file. The 365-day window is a strict superset of the 90-day window. So all-time counts appear smaller than 1-year counts.Breadth
The same base-table window is reused for three more Governance sections, all derived from the same base
combinedtable via_write_repo_summaries(..., period=None):repoactivity→repo_activity_overview.csv(active_recent,*_actions_recent).understaffed→maintainer_coverage_risk.csv(active_maintainers, derived fromstatus).loadshare→review_load_share.csv(load_recent).
Other
periods: Truesections (for exampleteams,teamrepo,tscrepo, andprofiles) rebuild every period file independently and do not mix an all-time base with a fixed 90-day window, so they do not show this defect. Fixing the base computation inpipelines/role_coverage.pyfixes all four affected Governance sections at once.File-Level Change Summary
src/hiero_analytics/pipelines/role_coverage.py: change the base-table_build_role_coverage(...)call inmain()to use all-time profiles forrecent_profilesandactive_within_days=None; keep the 90-dayrecent_by_repoonly for network charts; update docstrings/comments to match. Verify_write_repo_summaries(..., period=None)reads the correctedcombinedtable.tests/pipelines/test_role_coverage.py: add a regression test that asserts the all-time superset invariant (base*_recent≥_365d*_recent), that base counts include activity older than 90 days, and that all-timestatusmarks any ever-active holder active.src/hiero_analytics/dashboard_spec/governance.py: no change expected (columns unchanged). Change only if a later column rename is chosen.tests/contracts/test_output_contract.py: no change expected; run to confirm the contract still holds.
🚀 Next Steps
🤖 All AI agent prompts combined
Task: 1 Implement the fix for the all-time base table in `src/hiero_analytics/pipelines/role_coverage.py`. The goal: the base ("All time") `role_coverage` table must compute its `*_recent` count columns and `status` from all-time data, not from a fixed 90-day window. - In `main()`, find the base-table call `combined = _build_role_coverage(...)`. Change its `recent_profiles` argument to use the all-time per-repo profile set (`all_time_by_repo`) instead of the 90-day `recent_by_repo`. Change `active_within_days` to `None` for this base call so `status` marks a role holder active based on any recorded activity, not a 90-day cutoff. - Do not modify `_build_profile_sets()`'s 90-day `recent_by_repo` value; it must remain available and unchanged, because it still feeds the co-membership network charts (`_write_role_networks` → `build_active_membership`). - Do not modify the `for period in ACTIVITY_PERIODS:` loop that rebuilds the `_7d`, `_30d`, and `_365d` variants; these are already correct. - Verify that `_write_repo_summaries(combined, ..., period=None)` derives `repo_activity_overview.csv`, `maintainer_coverage_risk.csv`, and `review_load_share.csv` from the corrected `combined` table. If any of these three summaries independently recompute a 90-day window instead of reading from `combined`, apply the same all-time correction (all-time counts, `active_within_days=None`) to that computation. - Update the docstrings and comments in `_build_profile_sets`, `_build_role_coverage`, and the base-table section of `main()` to state that the base file's `*_recent` counts and `status` are all-time, and that the 90-day `ROLE_ACTIVE_DAYS` window now applies only to network charts. - Do not change the meaning or value of `ROLE_ACTIVE_DAYS` in `config/analysis.py`. It must keep governing the network-chart recency window and the per-repo `role_coverage.csv`/`role_promotion_candidates.csv` threshold. =============================================================================== Task: 2 Add regression tests and verify the output contract for the all-time fix made in Phase 1. - Extend `tests/pipelines/test_role_coverage.py` with a new test. Build a synthetic activity fixture that includes review/PR events older than 90 days but within 365 days, for at least one role holder in one repo. - Run `main()` against this fixture, then load `role_coverage_all.csv` and `role_coverage_all_365d.csv`. - Assert that, for each role holder and repo, every base `*_recent` value (`reviews_recent`, `prs_recent`, `merges_recent`, `issues_recent`, `labels_recent`) in `role_coverage_all.csv` is greater than or equal to the corresponding value in `role_coverage_all_365d.csv`. - Assert that the base `*_recent` counts equal the true all-time counts, including the events older than 90 days. - Assert that a role holder whose only activity is older than 90 days has `status` equal to `active` (not quiet) in the base file. - Do not rename or add any column in `dashboard_spec/governance.py` for the `repo`, `repoactivity`, `understaffed`, or `loadshare` sections; the fix changes only column values, not the schema. - Run `tests/contracts/test_output_contract.py` and confirm it still passes, since it derives its expectations from `SECTION_SPECS` at run time. - Only if a later design decision requires a column rename or addition, update the matching `SECTION_SPECS` entry in `dashboard_spec/governance.py`; otherwise leave it unchanged.💡 Iterate on the plan with:
`@coderabbitai` <feedback>Example Feedback - `@coderabbitai` You can skip phase 3. Add a simple unit test case for phase 2. - `@coderabbitai` For design choice 1 go ahead with option 3 and replan.
💬 Have feedback or questions? Drop into our discord!
- Make the base table fully all-time. Pass the all-time profile set as
/assign
CodeRabbit chat interactions are restricted to organization members for this repository. Ask an organization member to interact with CodeRabbit, or set
chat.allow_non_org_members: truein your configuration.CodeRabbit chat interactions are restricted to organization members for this repository. Ask an organization member to interact with CodeRabbit, or set
chat.allow_non_org_members: truein your configuration.we can creat an issue to solve this planning issue please @5affron
Reacted by Piyush Raj@exploreriii Sure, I’ll create the follow-up issue. I’ll first verify the proposed cause in the code, then add the implementation and regression-testing details there before starting the changes.
@5affron please confirm if you are still working on this, and if the issue is still relevant given the recent updates
assignment released due to staleness. Open to all once again
cc @sudo-anshul
on it, thanks for the ping @exploreriii! 🙌
The task
Problem:
Our tables have time filters: week, 1 month, 1 year, all time.
The all time filter is adding up to smaller values than the 1 year filter for example:
https://hiero-hackers.github.io/analytics/#tab=Governance&widget=repo
e.g. exploreriii has 148 all time reviews in the python SDK but 2247 1 year reviews.
This bug might be occuring elsewhere in the dashboard - we shoudl fix this.
What done looks like:
Identify the cause and breadth of the bug
Resolve so that the all time filter actually counts all time data
Modules involved / constraints:
How to work on this
/assignand wait to be assigned — unassigned PRs are closed automatically.@coderabbitai planfor a draft plan, then do your own investigation — docs/architecture.md maps the layers and their rules.🤖 AI: tools are welcome; verified work is required — you can explain every line and defend every design choice. See the AI policy. Fully automated bot PRs are closed.
Worth knowing about this repo before you design:
tests/<pkg>/test_<module>.py), and the output-contract test pins the pipeline output surface — if your change adds or renames outputs, update the contract deliberately.Before opening your PR:
Stuck? Comment here with what you've tried — see getting help.