Skip to content

feat(tracking): record project_path on parse failures - #3227

Open
bharvey42 wants to merge 1 commit into
rtk-ai:developfrom
bharvey42:feat/telemetry-parse-failure-project-path
Open

bharvey42 wants to merge 1 commit into
rtk-ai:developfrom
bharvey42:feat/telemetry-parse-failure-project-path

Conversation

@bharvey42

@bharvey42 bharvey42 commented Jul 26, 2026 •

Copy link
Copy Markdown

Problem

commands has been stamped with the cwd since project-scoped stats landed, but parse_failures was not. A failure could therefore only be related to other telemetry by bare timestamp, which is ambiguous on a machine running concurrent sessions in different projects — so "which project do rtk's filters keep breaking in?" was not answerable from the data.

Changes

  • Add parse_failures.project_path TEXT DEFAULT '', stamped by record_parse_failure via the same current_project_path_string() helper record uses. Call sites are unchanged.
  • Add get_parse_failure_summary_filtered() and wire --project through rtk gain --failures, using the same exact-or-subdirectory GLOB scoping as the commands queries. The unscoped failure log now labels each recent row with its project directory, since that view spans several projects.
  • Index (project_path, timestamp) to match idx_project_path_timestamp.

Migration handling

The column is added in run_schema_migrations(), and SCHEMA_VERSION goes from 2 to 3. v1.0.0 already stamps databases with user_version 2, so without the bump Tracker::new()'s version gate would skip the new statement on every machine that has run 1.0. test_gated_open_migrates_v2_db_to_parse_failures_project_path opens a v2-stamped DB through the gated path to pin that (it fails with the version left at 2).

Existing rows take the column DEFAULT rather than NULL, so readers never have to tell "old row" from "unknown project".

Privacy

No new field is reported upstream: telemetry sends parse_failures_24h and projects_count, both counts, and no raw path leaves the machine.

Verification

Rebased onto develop 8157d4d (v1.0.0). cargo test --all --no-fail-fast on Windows: every target passes except two known Windows-environment failures in files this PR doesn't touch (find_cmd::run_from_args_propagates_find_exit_status and stream::test_read_lines_lossy_preserves_lines_after_invalid_utf8. Clippy is clean.

On a copy of a production history.db reshaped to v1.0's schema (user_version 2, no parse_failures.project_path): all 17653 existing rows migrate to '', user_version becomes 3, the index is created and 1.0's own tables are untouched.

@CLAassistant

CLAassistant commented Jul 26, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@bharvey42
bharvey42 force-pushed the feat/telemetry-parse-failure-project-path branch from 9a47b56 to 66690ca Compare September 1, 2026 09:13
@bharvey42
bharvey42 force-pushed the feat/telemetry-parse-failure-project-path branch from 66690ca to 62b4476 Compare September 15, 2026 04:01
@bharvey42

Copy link
Copy Markdown
Author

Rebased onto current develop (5e0f92c) — conflict-free again, and simplified in the process.

develop has since done the schema unification this PR originally carried: Tracker::new() and new_in_memory() both run one run_schema_migrations(), gated by SCHEMA_VERSION in Tracker::new(). So this branch no longer ships its own migrate_schema() helper. Instead:

  • the parse_failures.project_path column + idx_pf_project_path_timestamp index are added inside run_schema_migrations(), and
  • SCHEMA_VERSION is bumped 1 → 2 so existing databases actually pick the migration up on their next open (with the gate, a new ALTER TABLE without a bump would never run against an already-stamped DB).

Recording (record_parse_failure stamps cwd), the --project filter for rtk gain --failures, the ParseFailureRecord.project_path field, and the README are as before. The legacy-schema migration test now drives run_schema_migrations() directly (renamed to test_schema_migration_adds_parse_failures_project_path).

Full gate on Windows 11: fmt clean, clippy zero warnings, 3599 passed / 0 failed. The 3 remaining failures (find_cmd, stream, hook_cmd) reproduce identically on pristine develop under Windows — pre-existing, unrelated.

@bharvey42

Copy link
Copy Markdown
Author

Friendly ping: this one is still waiting on workflow approval. The fork-PR CI run for 8ec0ecb is sitting at action_required, so no checks have run yet.

Re-checked today against develop c356374:

  • Merges with no conflicts.
  • cargo fmt --all -- --check and cargo clippy --all-targets are clean.
  • cargo test --all: 3946 passed, 2 failed. The two failures are find_cmd::run_from_args_propagates_find_exit_status and stream::test_read_lines_lossy_preserves_lines_after_invalid_utf8. Both also fail on develop without this PR on Windows. (This was on the GNU toolchain, with fix(build): use the GNU linker's stack flag on *-pc-windows-gnu #3239's build.rs applied so the test binaries link.)
  • develop's SCHEMA_VERSION is still 1, so the bump to 2 here still lines up.

Happy to rebase onto current develop if a fresh head is easier to review.

@bharvey42
bharvey42 force-pushed the feat/telemetry-parse-failure-project-path branch from 8ec0ecb to 5a306cd Compare October 14, 2026 02:44
`commands` has been stamped with the cwd since project-scoped stats landed,
but `parse_failures` was not. A failure could therefore only be related to
other telemetry by bare timestamp, which is ambiguous on a machine running
concurrent sessions in different projects — so "which project do rtk's
filters keep breaking in?" was not answerable from the data.

- Add `parse_failures.project_path TEXT DEFAULT ''`, stamped by
  `record_parse_failure` via the same `current_project_path_string()` helper
  `record` uses. Call sites are unchanged.
- Add `get_parse_failure_summary_filtered()` and wire `--project` through
  `rtk gain --failures`, using the same exact-or-subdirectory GLOB scoping as
  the `commands` queries. The unscoped failure log now labels each recent row
  with its project directory, since that view spans several projects.
- Index `(project_path, timestamp)` to match `idx_project_path_timestamp`.

The column is added in `run_schema_migrations()`, and SCHEMA_VERSION goes
from 2 to 3. v1.0.0 already stamps databases with user_version 2, so
without the bump `Tracker::new()`'s version gate would skip the new
statement on every machine that has run 1.0. A test opens a v2-stamped DB
through the gated path to pin that. Existing rows take the column DEFAULT
rather than NULL, so readers never have to tell "old row" from "unknown
project".

No new field is reported upstream: telemetry sends `parse_failures_24h` and
`projects_count`, both counts, and no raw path leaves the machine.

Verified on a copy of a production history.db reshaped to v1.0's schema
(user_version 2, no parse_failures.project_path): all 17653 existing rows
migrate to '', user_version becomes 3, the index is created and 1.0's own
tables are untouched. Stamping and `--failures --project` scoping are
covered by the unit tests.
@bharvey42
bharvey42 force-pushed the feat/telemetry-parse-failure-project-path branch from 5a306cd to b1a2d72 Compare October 14, 2026 02:47
@bharvey42

Copy link
Copy Markdown
Author

@KuSh, a gentle nudge on this one when you have a moment. It's small and self-contained (3 files, one commit), and it still merges cleanly onto today's develop (e05c7bd).

Re-checked today with this branch merged onto e05c7bd, on Windows x86_64-pc-windows-gnu: cargo test --all --no-fail-fast gives 4942 passed / 2 failed. The 2 failures are the known Windows baselines: #4610 (find_cmd) and #3730 (stream). Neither touches tracking. Both schema tests pass: test_schema_migration_adds_parse_failures_project_path and test_gated_open_migrates_v2_db_to_parse_failures_project_path.

One thing to watch whenever this is rebased: v1.0 took SCHEMA_VERSION = 2 for itself (classifier_cache + engine_attribution), so this PR's migration is 3. Git merges two sides' = 2 without a conflict. If the bump is lost in a rebase, a 1.0 database silently skips the migration. test_gated_open_migrates_v2_db_to_parse_failures_project_path covers that case: it fails at 2 and passes at 3.

The fork-PR CI run for b1a2d72 has been waiting on workflow approval since 10-14. Could someone approve it and take a look? If you'd rather it be sliced differently, or folded into something you already have open around tracking, I'm happy to do that.

@KuSh KuSh self-assigned this Nov 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants