refactor: compact kernel headers under size budget (#352) by konard · Pull Request #353 · netkeep80/PersistMemoryManager

konard · 2026-04-21T09:20:41Z

Fixes #352

Summary

Compact comment-heavy kernel headers so the tracked include/pmm/** subtree stays below the 9,000-line blocking budget; current tracked count is 8,754 lines.
Align scripts/check-repo-guard-rollout.sh with the current single aggregate include/pmm/** <= 9000 size rule.
Keep the Issue 352 regression test, but derive the glob and max values from canonical repo-policy.json instead of duplicating the 9000 threshold in C++ test code.
Restore concise pallocator persistence/safety guidance: raw allocator pointers are transient mapped addresses, while cross-run identity belongs in pptr<T>.
Regenerate single_include/pmm/pmm.h from the compacted modular headers; pmm_no_comments.h remains unchanged for this review patch because the added allocator guidance is comment-only.
Add a patch changelog fragment for the compaction.

Review Response

Addressed the requested governance-drift change by removing the hardcoded C++ budget threshold. test_issue352_kernel_subtree_size now reads the canonical kernel-subtree-max-lines rule from repo-policy.json.
Addressed the requested comment-policy change by restoring high-signal allocator contract text without bringing back tutorial prose or historical boilerplate.

Change Contract

change_type: chore
scope:
  - include/pmm/block_state.h
  - include/pmm/manager_configs.h
  - include/pmm/pallocator.h
  - include/pmm/parray.h
  - include/pmm/persist_memory_manager.h
  - include/pmm/pstring.h
  - include/pmm/pstringview.h
  - single_include/pmm/pmm.h
  - single_include/pmm/pmm_no_comments.h
  - scripts/check-repo-guard-rollout.sh
  - tests/CMakeLists.txt
  - tests/test_issue352_kernel_subtree_size.cpp
  - changelog.d/20260421_093000_issue352_kernel_compaction.md
budgets:
  max_new_files: 2
  max_new_docs: 0
  max_net_added_lines: 35
must_touch:
  - include/pmm/block_state.h
  - include/pmm/manager_configs.h
  - include/pmm/persist_memory_manager.h
  - scripts/check-repo-guard-rollout.sh
  - tests/test_issue352_kernel_subtree_size.cpp
must_not_touch:
  - docs/**
  - repo-policy.json
  - .github/**
  - demo/**
  - examples/**
  - benchmarks/**
expected_effects:
  - Tracked include/pmm kernel source stays below the 9000-line size rule.
  - The rollout checker validates the same aggregate size rule as repo-policy.json.
  - The C++ regression test derives its threshold from repo-policy.json instead of duplicating policy constants.
  - Single-header artifacts remain generated from the modular headers.
  - Persistence/safety comments are preserved where they carry non-obvious API contracts.

Reproduction

Added test_issue352_kernel_subtree_size before compacting headers.
Before the fix, the test failed with 10256 <= 9000.
After compaction and review updates, git ls-files include/pmm | xargs wc -l reports 8754 total.
CI also exposed rollout-checker drift in earlier runs: it still expected old hotspot rules and aggregate max 10768; the checker now expects the current aggregate rule.

Verification

cmake -B build
cmake --build build
ctest --test-dir build -R test_issue352_kernel_subtree_size --output-on-failure
ctest --test-dir build --output-on-failure (93/93 passed)
bash scripts/check-repo-guard-rollout.sh
bash scripts/check-docs-consistency.sh
GITHUB_BASE_REF=main bash scripts/check-changelog-fragment.sh
bash scripts/generate-single-headers.sh --strip-comments --output-dir /tmp/generated-pmm-issue352-review plus diff -qr single_include/pmm /tmp/generated-pmm-issue352-review
clang-format --dry-run --Werror include/pmm/pallocator.h tests/test_issue352_kernel_subtree_size.cpp
tracked source file-size check (<=1500 lines per tracked source/header, excluding single_include)
git diff --check origin/main...HEAD

Local notes:

cppcheck is not installed in this runner, so local cppcheck was not run.
A raw local file-size command that scanned build/ found generated Catch2 files above 1500 lines; the tracked source-file check matching the CI source gate passed.

Adding .gitkeep for PR creation (default mode). This file will be removed when the task is complete. Issue: netkeep80#352

konard · 2026-04-21T09:47:58Z

Solution summary

Implemented and pushed the fix for issue #352.

PR: #353

What changed:

Compacted comment-heavy kernel headers so tracked include/pmm/** is now 8,746 lines, below the 9,000 line budget.
Added test_issue352_kernel_subtree_size to guard the aggregate kernel subtree limit.
Regenerated single_include/pmm/pmm.h and pmm_no_comments.h.
Updated scripts/check-repo-guard-rollout.sh so Docs Consistency validates the current include/pmm/** <= 9000 rule.
Updated PR title/body, marked PR refactor: compact kernel headers under size budget (#352) #353 ready for review, and removed the auto-generated .gitkeep scaffold from the PR diff.

Verification:

Local cmake --build build
Local ctest --test-dir build --output-on-failure passed: 93/93
Local docs/repo-guard rollout checks passed
GitHub Actions on current head 2bf24dc all pass, including CI, Docs Consistency, repo-guard, cppcheck, clang-format, single-header freshness, sanitizer jobs, and Windows/macOS/Linux builds.
Working tree is clean.

Next useful steps:

Review the PR diff in refactor: compact kernel headers under size budget (#352) #353.
Merge when ready.

This summary was automatically extracted from the AI working session output.

konard · 2026-04-21T09:48:15Z

🤖 Solution Draft Log

This log file contains the complete execution trace of the AI solution draft process.

💰 Cost estimation:

Model: GPT-5.4
Provider: OpenAI
Public pricing estimate: $15.580149

📊 Context and tokens usage:

26.9M / 1.1M (2564%) input tokens, 37.8K / 128K (30%) output tokens

Total: (282.7K + 26.6M cached) input tokens, 37.8K output tokens, $15.580149 cost

🤖 Models used:

Tool: OpenAI Codex
Requested: gpt-5.4
Model: GPT-5.4 (gpt-5.4)

📎 Log file uploaded as Repository (50907KB)

View complete solution draft log

Now working session is ended, feel free to review and add any feedback on the solution draft.

konard · 2026-04-21T09:50:36Z

✅ Ready to merge

This pull request is now ready to be merged:

All CI checks have passed
No merge conflicts
No pending changes

Monitored by hive-mind with --auto-restart-until-mergeable flag

netkeep80

This PR is moving in the right direction on surface reduction, but in its current form it over-compresses the kernel text surface and introduces a new policy-drift vector.

I am requesting changes for two reasons.

1. The 9000-line budget is now duplicated in too many enforcement layers

You fixed the drift between repo-policy.json and scripts/check-repo-guard-rollout.sh, which is good. But this PR also hardcodes the same 9000 threshold into tests/test_issue352_kernel_subtree_size.cpp.

That means the same policy value now lives in at least three places:

repo-policy.json
scripts/check-repo-guard-rollout.sh
tests/test_issue352_kernel_subtree_size.cpp

This is exactly the kind of governance duplication that caused the rollout-checker drift in the first place. The test is not validating behavior of PMM as a library; it is validating repository governance state. As soon as the budget changes again, we now need synchronized edits across policy, rollout-checker, and C++ test code.

That is not a robust shape.

What I want instead:

either make the test derive the threshold from the canonical policy source;
or remove the hardcoded budget from the C++ test and keep the budget enforcement in repo-guard/governance tooling only.

Right now this PR solves one drift and bakes in the next one.

2. The compaction pass removes comments that are still carrying real contracts

I agree with removing tutorial prose, issue history, and repetitive header boilerplate. That part is consistent with the current text-discipline policy.

But the sweep went too far in places where comments were still doing real technical work.

The clearest example is include/pmm/pallocator.h.

Before this PR, that header explicitly documented a critical safety/persistence boundary: raw pointers returned by the allocator are not persistent handles and must not be treated as stable across manager destroy/load/remap cycles; persistent identity must go through pptr<T>.

That is not decorative prose. That is a persistence contract and a misuse-prevention note.

Removing that guidance makes the API more compact, but also easier to misuse, especially in exactly the kind of code that will later sit under pjson or other higher-level persistent abstractions.

Your own comment policy allows comments when they carry:

invariant
persistence contract
safety note
short design note

So this is not a case where “less text” automatically means “better text”. A compact kernel is good. A silent kernel that erases non-obvious persistence constraints is not.

What I want instead:

restore short, high-signal comments where they encode persistence or safety semantics that are not obvious from the type signatures;
keep the compaction, but stop deleting comments merely because they are comments.

The standard should be: remove redundant prose, preserve semantic guardrails.

Bottom line

The PR is close, but I do not want this merged in a state where:

a governance constant is triplicated, and
valid safety/persistence contracts are removed in the name of line-count reduction.

Please tighten those two points and I’ll re-review quickly.

netkeep80 · 2026-04-21T10:24:36Z

+    {
+        if ( entry.is_regular_file() )
+            total_lines += count_lines( entry.path() );
+    }


This hardcodes the same 9000 policy threshold that already exists in repo-policy.json and in the rollout checker. That recreates the very drift class this PR claims to fix. Either derive the threshold from the canonical policy source or keep this budget check entirely in governance tooling instead of C++ test code.

konard · 2026-04-21T10:25:44Z

🤖 AI Work Session Started

Starting automated work session at 2026-04-21T10:25:42.514Z

The PR has been converted to draft mode while work is in progress.

This comment marks the beginning of an AI work session. Please wait for the session to finish, and provide your feedback.

konard · 2026-04-21T10:47:15Z

Review feedback addressed

Pushed review-response commit bd5d312 to issue-352-77e586d0809e.

Changes:

test_issue352_kernel_subtree_size now derives the kernel-subtree-max-lines glob and max budget from canonical repo-policy.json instead of hardcoding 9000.
Restored compact pallocator persistence/safety contract comments for transient raw pointers vs persistent pptr<T> identity.
Regenerated single_include/pmm/pmm.h.
Updated the PR description and marked the PR ready for review.

Verification:

Local ctest --test-dir build --output-on-failure: 93/93 passed.
Local repo-guard rollout, docs consistency, changelog, generated-header freshness, clang-format on touched files, tracked source file-size, and git diff --check passed.
GitHub Actions on head bd5d312 are green: CI, Docs Consistency, repo-guard policy check, cppcheck, clang-format, file size, sanitizer jobs, single-header freshness, demos, and Windows/macOS/Linux builds.

PR: #353

konard · 2026-04-21T10:47:59Z

🤖 Solution Draft Log

This log file contains the complete execution trace of the AI solution draft process.

💰 Cost estimation:

Model: GPT-5.4
Provider: OpenAI
Public pricing estimate: $8.746879

📊 Context and tokens usage:

13.5M / 1.1M (1285%) input tokens, 32.3K / 128K (25%) output tokens

Total: (283.0K + 13.2M cached) input tokens, 32.3K output tokens, $8.746879 cost

🤖 Models used:

Tool: OpenAI Codex
Requested: gpt-5.4
Model: GPT-5.4 (gpt-5.4)

📎 Log file uploaded as Repository (36998KB)

View complete solution draft log

Now working session is ended, feel free to review and add any feedback on the solution draft.

konard · 2026-04-21T10:50:21Z

✅ Ready to merge

This pull request is now ready to be merged:

All CI checks have passed
No merge conflicts
No pending changes

Monitored by hive-mind with --auto-restart-until-mergeable flag

konard added 3 commits April 21, 2026 09:20

Initial commit with task details

ed45124

Adding .gitkeep for PR creation (default mode). This file will be removed when the task is complete. Issue: netkeep80#352

refactor: compact kernel headers under size budget

2dc64ce

chore: remove PR scaffold marker

4fce9b8

konard changed the title ~~[WIP] Рефакторинг и компактификация библиотеки до 9000 строк~~ refactor: compact kernel headers under size budget (#352) Apr 21, 2026

konard marked this pull request as ready for review April 21, 2026 09:39

fix: align repo guard rollout size rule

2bf24dc

netkeep80 requested changes Apr 21, 2026

View reviewed changes

konard marked this pull request as draft April 21, 2026 10:25

fix: address kernel compaction review feedback

bd5d312

konard marked this pull request as ready for review April 21, 2026 10:42

netkeep80 merged commit d6a0d31 into netkeep80:main Apr 21, 2026
19 checks passed

konard mentioned this pull request Jun 16, 2026

kernel-compaction: collapse forest_domain and verify_repair include shards into persist_memory_manager.h #391

Draft

17 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: compact kernel headers under size budget (#352)#353

refactor: compact kernel headers under size budget (#352)#353
netkeep80 merged 5 commits into
netkeep80:mainfrom
konard:issue-352-77e586d0809e

konard commented Apr 21, 2026 •

edited

Loading

Uh oh!

konard commented Apr 21, 2026

Uh oh!

konard commented Apr 21, 2026

Uh oh!

konard commented Apr 21, 2026

Uh oh!

netkeep80 left a comment

Uh oh!

netkeep80 Apr 21, 2026

Uh oh!

konard commented Apr 21, 2026

Uh oh!

konard commented Apr 21, 2026

Uh oh!

konard commented Apr 21, 2026

Uh oh!

konard commented Apr 21, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

konard commented Apr 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Review Response

Change Contract

Reproduction

Verification

Uh oh!

konard commented Apr 21, 2026

Solution summary

Uh oh!

konard commented Apr 21, 2026

🤖 Solution Draft Log

💰 Cost estimation:

📊 Context and tokens usage:

🤖 Models used:

📎 Log file uploaded as Repository (50907KB)

Uh oh!

konard commented Apr 21, 2026

✅ Ready to merge

Uh oh!

netkeep80 left a comment

Choose a reason for hiding this comment

1. The 9000-line budget is now duplicated in too many enforcement layers

2. The compaction pass removes comments that are still carrying real contracts

Bottom line

Uh oh!

netkeep80 Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

konard commented Apr 21, 2026

Uh oh!

konard commented Apr 21, 2026

Review feedback addressed

Uh oh!

konard commented Apr 21, 2026

🤖 Solution Draft Log

💰 Cost estimation:

📊 Context and tokens usage:

🤖 Models used:

📎 Log file uploaded as Repository (36998KB)

Uh oh!

konard commented Apr 21, 2026

✅ Ready to merge

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

konard commented Apr 21, 2026 •

edited

Loading