Skip to content

Solution for gh-ppx_minidebug-84-followup#100

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

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

Conversation

@lukstafi
Copy link
Copy Markdown
Owner

@lukstafi lukstafi commented Mar 2, 2026

Solution from coder for feature: gh-ppx_minidebug-84-followup

lukstafi added 3 commits March 2, 2026 23:16
…ized entry decoding and shared DAG propagation in client/query.ml, added public find_scope_headers API, split extract-search logic into focused helpers, added deterministic metadata-based test DB fixture utility, migrated affected tests, and added focused DAG quiet-path stop regression test. Verified with dune build, targeted runtests, and full dune runtest.
@lukstafi lukstafi merged commit 6c8f648 into main Mar 2, 2026
6 checks passed
@lukstafi lukstafi deleted the gh-ppx_minidebug-84-followup branch March 2, 2026 22:41
@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...

  • Move query row decoding and query SQL fragments in client/query.ml into a small internal submodule (Entry_row) with one canonical column order constant and one decode function, then assert column-count/shape in one place to prevent drift across query call sites.
  • Split populate_search_results in client/query.ml the same way we split extract mode: isolate stream scanning, retroactive highlight handling, and propagation orchestration. It is still too monolithic and hard to reason about for edge cases.
  • Replace the mutable results_table : (int * int, bool) Hashtbl.t boolean flag with an explicit internal type like highlight_kind = Match | Ancestor; that would make search/extract semantics clearer and reduce accidental misuse at call sites.
  • Add a shared internal utility in client/query.ml for "highlight all headers for scope" returning inserted keys, and use it in both search and extract paths so highlight insertion/removal bookkeeping is not duplicated.
  • Add focused unit-level tests for propagation behavior without requiring full end-to-end DB creation (e.g. a tiny synthetic fixture DB checked into test/fixtures/), then keep only a small number of full integration tests in test/*.ml.
  • Add API-level docs in client/query.mli for DAG semantics (find_scope_header first-only vs find_scope_headers all-parents) and quiet-path stopping rules, because these are subtle and currently mostly implicit in code.
  • Consolidate test fixture helpers into a dedicated test utility library module (not just latest-run lookup), including reusable helpers for "find duplicated ancestor", "collect header keys", and deterministic DB path resolution.

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