Improve build time. Add a new header file holding trivial typedefs instead of using types.hpp

[vasil-pashov]

Reference Issues/PRs

What does this implement or fix?

According to ClangBuildAnalyzer types.hpp is one of the most included files in the codebase. It contains trivial typedefs such as timestap = uint64_t. These things cannot be fwd declared thus many files include this so that they can use the typedef of a POD. In order to improve the build time a new file typedefs.hpp holding only these simple typedefs is added. Also because of how types.hpp was used it leaked lots of namespaces which lead to compilation errors because of ambiguous namespaces. No functional changes are present in the PR only namespaces and includes are changed.

Any other comments?

Checklist

Checklist for code changes...

• Have you updated the relevant docstrings, documentation and copyright notice?
• Is this contribution tested against all ArcticDB's features?
• Do all exceptions introduced raise appropriate error messages?
• Are API changes highlighted in the PR description?
• Is the PR labelled as enhancement or bug so it appears in autogenerated release notes?

[claude]

ArcticDB Code Review Summary - full content below in inline review comments. Issues: 1) Ill-formed nested namespace in entity/protobuf_mappings.hpp line 13 (namespace arcticdb::proto inside namespace arcticdb block - ill-formed C++17). 2) Commented-out fmt::formatter for StreamId in types-inl.hpp line 165 (will break fmt::format of StreamId for TUs using typedefs.hpp without types.hpp). 3) Duplicate field_proto declaration in entity/types_proto.hpp line 43. 4) Missing trailing newlines in 6 files. Positives: noexcept on move ctor, maybe_unused attribute, pragma once fix. Overall: sound approach, well-motivated. Fix bugs 1 and 2 before merge.

[claude]

line1
line2

[claude]

ArcticDB Code Review Summary

PR #3008 — Improve build time: add a new header file holding trivial typedefs instead of using types.hpp
Event: synchronize | Full diff reviewed (delta was empty)

---

API & Compatibility

• ✅ No breaking changes to public Python API (Arctic, Library, NativeVersionStore)
• ✅ On-disk format unchanged; protobuf schema backwards-compatible
• ✅ Key types and ValueType enum unchanged (additive only)

Memory & Safety

• ✅ RAII, no use-after-move, no buffer overflows, GIL unchanged, no accidental copies

Correctness

• ✅ All ValueType/KeyType switch statements exhaustive (N/A — pure refactoring)

• ❌ fmt::formatter<StreamId> removed — output format regresses in log/error messages

  The custom formatter in entity/types-inl.hpp (lines 162–176) was deleted. It used std::visit to print the variant value directly (e.g., my_symbol, 42). With it gone, calls to fmt::format("{}", stream_id) fall back to fmtlib v10's built-in std::variant formatter from <fmt/std.h> (included transitively via preconditions.hpp), which wraps the output as variant(my_symbol) or variant(42).

  Affected callers include:

  • storage/file/file_store.hpp:103,137 — file path construction
  • storage/mongo/mongo_client.cpp:340,402,407 — Mongo document keys
  • version/snapshot.cpp:187,191 and version/version_functions.hpp:274,288,303 — debug log messages
  • version/version_store_api.cpp:670,1321,1397 — user-visible error messages

  Fix: move the formatter to typedefs.hpp or output_format.hpp.

Code Quality

• ✅ PR fixes the pre-existing duplicate field_proto declaration in entity/types_proto.hpp
• ⚠️ UnicodeType = wchar_t is now defined in both typedefs.hpp and types.hpp (minor redundancy; valid C++ but incomplete cleanup)
• ⚠️ A previous Claude inline comment on entity/protobuf_mappings.hpp:26 incorrectly flagged the namespace syntax as ill-formed. The code uses namespace arcticdb { namespace proto { ... } }, which is valid standard C++. No action needed from the author.

Testing

• ✅ Test files updated to use qualified names (stream::TimeseriesIndex, etc.)
• ❌ No test covers the StreamId formatting regression (output changed silently)

Build & Dependencies

• ✅ typedefs.hpp is header-only — no CMakeLists.txt entry required
• ✅ Cross-platform: ssize_t Windows guard correctly added in typedefs.hpp
• ⚠️ 179 files changed — a full build verifying no transitive-include breakage is important

Security

• ✅ No hardcoded credentials, no input validation bypassed, no buffer overflows

PR Title & Description

• ✅ Title and description are clear; motivation is well-explained (ClangBuildAnalyzer evidence)
• ❌ PR is not labelled (suggest enhancement)

---

Summary: Well-motivated build-time improvement. The one substantive concern is the deleted fmt::formatter<StreamId>: code will still compile (fmtlib v10 handles std::variant) but log and user-visible error messages containing a StreamId will silently degrade from my_symbol to variant(my_symbol). This should be fixed before merge.