Skip to content

compass: cite code by symbol, not line number, and drop category_counts - #499

Merged
jgong5 merged 1 commit into
feature/atomcompass_newfrom
compass/issue-491-symbol-cites
Sep 29, 2026
Merged

jgong5 merged 1 commit into
feature/atomcompass_newfrom
compass/issue-491-symbol-cites

Conversation

@jgong5

@jgong5 jgong5 commented Sep 29, 2026

Copy link
Copy Markdown
Owner

Closes #491

Text edits plus one deleted helper. No behaviour change. Under the #489 rule, code is cited by path and symbol, never by line number.

What changed

Every prose file.py:NNN citation under atom/compass/, tests/compass/ and scripts/compass/ (.py files) was resolved against the cited file at 8ac0dd28e and rewritten to a path and symbol (for example EngineCore.__init__ in atom/model_engine/engine_core.py). That includes the bare :NNN follow-on references in the same sentences. Where a sentence only carried the line number, the line number was dropped and the sentence kept its content. None of the citations turned out to be stale: each cited line was still inside the symbol the sentence describes.

Item counts in the comments I touched were dropped ("Fourteen dispatched names", "seven belong to", "three of these twelve", "Nine attributes", "broadcast five times"). Counts that describe a shape a caller unpacks ("unpacks three values", "fixes its four keys") were kept, and so were counts that a test in the same place asserts.

Per file

  • atom/compass/runner/overrides.py: module docstring (reply contract), the RPC_SURFACE comment block and its per-entry comments, and the _build_and_load_model, _estimate_cudagraph_overhead, get_num_blocks, capture_cudagraph and forward docstrings now cite AsyncIOProc.busy_loop, EngineCore.__init__, EngineCore._process_engine_step_inner, DecodeEngineCore, PPEngineCoreProc.*, Scheduler.postprocess, MemoryManagerMixin.*, LLMEngine.__init__, Config.__post_init__, StateRuntime.from_wire, RapidServeModelRunner.* and PPStageTransport.* by name.
  • atom/compass/runner/__init__.py: the exit and process_kvconnector_output bullets name the broadcasting methods.
  • atom/compass/runner/model_runner.py: the AsyncIOProc.__init__ comment.
  • atom/compass/runner/step_output.py: the "a step that produces nothing" paragraph describes the Scheduler.postprocess and Scheduler.schedule reads by what they do instead of by line.
  • atom/compass/memory/readings.py: the module docstring, and three runtime Term / MemoryRefusal strings, now name ModelRunner.get_num_blocks, ModelRunner._piecewise_per_token_bytes, ModelRunner.__init__ and RotaryEmbedding.
  • atom/compass/memory/graph_pool.py: the module docstring, constant comments, two docstrings and three runtime Term strings now name ModelRunner._estimate_cudagraph_overhead / _piecewise_per_token_bytes.
  • atom/compass/artifacts/matrix.py: the module docstring no longer says test_the_matrix_is_the_documents_table (deleted in compass(tests): delete the tests that assert prose (#489) #490) opens the design document and checks MATRIX against it. The design-doc path went with it. The "two sevens" count sentence was also dropped.
  • atom/compass/audit/sync_scan.py: deleted category_counts. Whole-tree grep (including scripts/ and tools/) found no caller; its only caller was the test compass(tests): delete the tests that assert prose (#489) #490 deleted. load_inventory, which it called, is kept because tests/compass/test_sync_inventory.py uses it.
  • tests/compass/test_runner_rpc_surface.py: the docstrings from compass(tests): delete the tests that assert prose #492's review comment, plus the _arity docstring and one comment, now cite DPEngineCoreProc._execute_dummy_batch, PPEngineCoreProc._pp_head_step / _downstream_busy_loop, LLMEngine.stop_profile, BlockManager.__init__ and Scheduler._warn_if_unschedulable.
  • tests/compass/test_runner_step_semantics.py, test_kv_budget.py, test_kv_budget_engine.py, test_capture_real_model.py: docstrings and comments rewritten the same way. In test_capture_real_model.py the :1115 references now say "site one", which SITE_ONE pins.

Remaining [a-z_]+\.py:[0-9]+ matches, and why they stay

Over the .py files of the three trees, every remaining match is a functional string:

  • tests/compass/test_capture_real_model.py: ROW_PARALLEL, VOCAB_EMBEDDING, VOCAB_LM_HEAD, SAMPLER, SITE_ONE, SITE_TWO, SITE_THREE, BUFFER_INIT and the keys of EXPECTED_HOST_RESOLUTIONS. These are recorded frame identifiers that the tests compare the capture's output against.
  • tests/compass/test_detect_loose_refusals.py: the expected detector output lines (atom/compass/opaque.py:3 ..., tests/compass/sample.py:6 ..., and so on) over a fixture tree.

Left alone because they are outside this issue's file set:

  • atom/compass/design/*.md (the PDES doc PRs under compass(design): revise the design documents to the PDES time model #443 rewrite them), scripts/compass/README.md and atom/compass/audit/sync_sites.json, as the brief says.
  • scripts/compass/regen_gpu_gate_triggers.sh and the header it generates into scripts/compass/gpu_gate_triggers.txt cite test_moe_dp_token_capacity.py:39 and tests/test_mla_index_cache.py:99-100 in comments. They are prose, but they are not .py files, and the .txt is generated from the .sh, so the two would have to change together. If they should be fixed, that belongs in a follow-up.

Gate

Full CPU gate (scripts/compass/gate_cpu.sh) on node 18 (xiaobizh_n18_cpu). Both trees were staged by git archive with stamps, md5-verified, and run one after the other, with PYTHONPATH set to each root and import atom checked to resolve under it:

side commit result
branch bab1a1eb9 GATE_CPU_RC=0 PASSED, 5213 passed, 155 skipped, 3 xfailed
control 8ac0dd28e GATE_CPU_RC=0 PASSED, 5213 passed, 155 skipped, 3 xfailed

The pass counts are equal, as expected: no test was added or removed. This PR fixes prose and deletes an uncalled function, so no mutation or pin applies.

black --check and ruff check are clean on all changed files.

🤖 Generated with Claude Code

Rewrite every prose `file.py:NNN` citation in comments, docstrings and
runtime messages under atom/compass/ and tests/compass/ to a path and
symbol, or delete it where the line number was its only content. The
file:line strings that tests compare against recorded frames or detector
output stay as they are.

Also drop the matrix.py docstring paragraphs that described a deleted
test, drop item counts in the comments touched, and delete
sync_scan.category_counts, which has no caller left.

No behaviour change.

Closes #491

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@jgong5

jgong5 commented Sep 29, 2026 •

Copy link
Copy Markdown
Owner Author

This review is agent-authored.

Review cycle 1

Verdict: APPROVE

Head covered: bab1a1eb90497ba416595c0352e4310f3383d96b (base feature/atomcompass_new at 8ac0dd28e). Diff read in full: 13 files, text edits plus the category_counts deletion.

1. No behaviour change

  • The code-bearing changes are the deleted category_counts and string literals. Every other hunk is a comment or docstring.
  • The changed runtime strings are Term source/note fields in atom/compass/memory/graph_pool.py (reserves) and atom/compass/memory/readings.py (ModelTerms), plus the what text of the device_readings MemoryRefusal. Term.source and Term.note are only read by Reading.table() in atom/compass/memory/terms.py, which renders them. Nothing parses a line number out of them, splits them or keys on them.
  • Tests that assert on these outputs match on substrings the PR did not touch. For example test_memory_compare.py asserts "reserves nothing" in rendered, and the eager graph_pool note still contains "reserves nothing for one". No match= or in str(...) in tests/compass names any rewritten fragment.
  • Artifact digests don't cover this text. atom/compass/artifacts/keys.py hashes the key's canonical(), store.py hashes payload files and spec/machine.py hashes the spec. No artifact module imports readings, graph_pool or Term. memory_readings appears there only as an artifact kind name.
  • No checked-in fixture carries the old strings. Outside design/, the only non-.py file that still matches engine_core.py:, model_runner.py: and similar patterns is atom/compass/audit/sync_sites.json, which is exempt.

2. Citation accuracy

I resolved every old file.py:NNN in the diff to its enclosing class and function at 8ac0dd28e with an ast walk. atom/model_engine, model_ops, rollout, distributed and utils have no diff between base and head. Every rewritten symbol matches, including the less obvious ones:

  • engine_core.py:749 resolves to DPEngineCoreProc._execute_dummy_batch.
  • engine_core.py:203 resolves to EngineCore._freeze_after_startup.
  • engine_core.py:500 resolves to EngineCore._dispatch_idle_offload_work.
  • engine_core.py:1109 resolves to DecodeEngineCore.__init__, and :1264 to DecodeEngineCore._process_engine_step.
  • pp_engine_core.py:232 resolves to PPEngineCoreProc._dispatch_connector_only_batch.
  • rollout/memory_manager.py:176/183 resolves to MemoryManagerMixin._resume_kv_cache, and :209 to _recapture_cudagraphs_if_needed.
  • config.py:1730 resolves to Config.__post_init__, and llm_engine.py:140 to LLMEngine.__init__.
  • llm_engine.py:300 resolves to LLMEngine.stop_profile, and scheduler.py:1364 to Scheduler._warn_if_unschedulable.
  • model_runner.py:3628 resolves to ModelRunner._piecewise_per_token_bytes, :4112 to ModelRunner.capture_cudagraph and :4272 to RapidServeModelRunner.get_num_blocks.
  • rotary_embedding.py:58 resolves to RotaryEmbedding._compute_inv_freq, and :39-49 to RotaryEmbedding.__init__.
  • pp_transport.py:141/114 resolves to PPStageTransport.send_tokens / recv_tokens.
  • aiter_attention.py:1115 is in prepare_decode, and forward_context.py:444 is in ForwardMode.assert_shape_contract.

The prose paraphrases in step_output.py are also right:

  • The prefix-hash need_placeholder use is at 2502, inside if not seq.prefix_hashes_published.
  • The placeholder loop at the end checks for RUNNING sequences that are not partial prefills.
  • The now_partial update sits at the top of Scheduler.postprocess.

block_manager.py:162 is enabled = self.enable_prefix_caching and self.num_state_slots > 0, which matches "decides there whether state checkpoints are enabled".

Non-blocking precision nits in atom/compass/runner/overrides.py:

  • NonAllocatingRunner.get_num_blocks docstring: "(_kv_budget_extra_reserve)" points at the four-margin reserve, but both ModelRunner and RapidServeModelRunner define that name. The very next sentence uses the same bare name for the base's zero. RapidServeModelRunner._kv_budget_extra_reserve would remove the ambiguity.
  • RPC_SURFACE: "async_proc_aggregation" # ... in EngineCore could name EngineCore._poll_kv_transfer_progress. "process_kvconnector_output" # EngineCore does not wait could name EngineCore._dispatch_idle_offload_work, the line it used to cite. flush_pp_send could name PPEngineCoreProc._head_busy_loop. As written the comments are accurate, just vague.

3. Kept matches are functional

Over the .py files of the three trees, grep -rnE '[a-z_]+\.py:[0-9]+' at the head matches only two files:

  • tests/compass/test_capture_real_model.py: ROW_PARALLEL, VOCAB_EMBEDDING, VOCAB_LM_HEAD and SAMPLER are keys of the expected collective map. SITE_ONE/TWO/THREE are compared by assert site(...) == SITE_*. BUFFER_INIT is compared by assert init["source"] == BUFFER_INIT. EXPECTED_HOST_RESOLUTIONS is compared by host_resolutions_by_line(at) == EXPECTED_HOST_RESOLUTIONS.
  • tests/compass/test_detect_loose_refusals.py: expected detector output over a fixture tree, asserted with in out.

All of these are functional, and the PR body lists them correctly.

4. category_counts is dead

grep -rn category_counts atom tests scripts tools at the head returns nothing. At the base, the definition was the only occurrence. load_inventory stays, correctly: test_sync_inventory.py calls it.

5. Staleness and lint

  • The added lines contain no design-doc references (D\d+, T\d+, W\d.\d, "principle", "Gate N", 0N_*.md), and none of them carry a :NNN line reference.
  • The matrix.py docstring loses the design-doc path and the "two sevens" count.
  • Remaining counts in touched sentences describe a code shape rather than the size of a list: "reads four keys", "unpacks three values", "four safety margins" (4 * safety_margin).
  • black --check and ruff check are clean on all 13 changed files. import atom resolves to the worktree.

6. Gate claim

Branch and control both report GATE_CPU_RC=0, 5213 passed. I trust that without a re-run on node 18. The diff adds and removes no test, and changes no expression except one deleted function with no caller. The runtime strings it changes are rendered only and not asserted on (section 1). Equal counts are therefore the only possible outcome, and the table shows them. A local tests/compass -q -x run at the head, in the dev container with import atom resolving to the worktree, finished green: 1263 passed. That tier is smaller than the node-18 gate, so it corroborates the claim but does not replace it.

7. regen_gpu_gate_triggers.sh follow-up

Non-blocking. scripts/compass/regen_gpu_gate_triggers.sh has two blocks that cite line numbers: the header comment around the topK bullet, and the heredoc that generates the .txt header. scripts/compass/gpu_gate_triggers.txt repeats the second one.

  • Both are stated as measured at 236abfd9a, so they don't drift the way a bare line number does.
  • The fix itself is cheap. The symbols are tests/model_ops/test_moe_dp_token_capacity.py::test_dpa_capacity_fits_gathered_topk_metadata (the cite also lacks the model_ops/ directory) and tests/test_mla_index_cache.py::test_model_runner_local_total_layers_adds_mtp_only_on_drafter_stage. I resolved both at 236abfd9a.
  • The cost is the .txt. It says "do not hand-edit", so the edit means re-running the regen script, which can change entries if the tree has moved.

Leaving it for a follow-up is reasonable.

8. Merge-tree

The fresh tip of fork/feature/atomcompass_new is 8ac0dd28e, unchanged. git merge-tree --write-tree 8ac0dd28e bab1a1eb9 gives d381de797, the same tree as the head. It merges clean.

9. ponytail-review

  • tests/compass/test_runner_step_semantics.py:L84-86: shrink. The docstring now takes three lines to say "in the engine's own order. / (blank) / The order is EngineCore._process_engine_step_inner's." One line does it: "ATOM's engine step, run to completion, in EngineCore._process_engine_step_inner's order." That wraps to two lines at 88 columns, one fewer than now.
  • atom/compass/audit/sync_scan.py: category_counts deleted, which is the right cut.

net: -1 lines possible.

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