Skip to content

Solution for gh-ppx_minidebug-84#99

Merged
lukstafi merged 3 commits intomainfrom
gh-ppx_minidebug-84
Mar 2, 2026
Merged

Solution for gh-ppx_minidebug-84#99
lukstafi merged 3 commits intomainfrom
gh-ppx_minidebug-84

Conversation

@lukstafi
Copy link
Copy Markdown
Owner

@lukstafi lukstafi commented Mar 2, 2026

Solution from coder for feature: gh-ppx_minidebug-84

@lukstafi lukstafi merged commit 730636a into main Mar 2, 2026
6 checks passed
@lukstafi
Copy link
Copy Markdown
Owner Author

lukstafi commented Mar 2, 2026

Refactoring Suggestions

Post-merge retrospective: what would we do differently if starting from scratch?

Refactoring Suggestions

If starting from scratch, I would...

  • Factor duplicated entry decoding into a single helper in client/query.ml (e.g., entry_of_stmt) used by search_entries, find_entry, find_scope_header, streaming search, and extract-search paths. This would remove repeated Sqlite3.column mapping blocks and reduce regression risk when entry schema handling changes.
  • Introduce a shared DAG-aware propagation helper in client/query.ml used by both populate_search_results and populate_extract_search_results (including direct-parent insertion + ancestor traversal + quiet-path stop semantics). The two code paths currently duplicate subtle propagation behavior and can drift.
  • Add a public query API for "all headers for child scope" in client/query.mli/client/query.ml (find_scope_headers), and migrate internal callers from ad-hoc SQL fragments. This would make DAG semantics explicit instead of relying on hidden local helpers.
  • Split populate_extract_search_results in client/query.ml into smaller units: candidate enumeration, dedup bookkeeping, highlight insertion, and propagation. The current monolithic function mixes concerns, making edge-case reasoning difficult.
  • Add a deterministic regression test fixture utility in test/ for resolving latest run DB via metadata (*_meta.db), so tests do not hardcode _1.db and do not depend on filesystem cleanup state.
  • Add one focused test that validates quiet-path stopping behavior in DAG propagation for both search and extract modes, to lock down the intended semantics when a duplicated ancestor matches quiet-path.

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.

1 participant