lablup / lablup/mlxcel

docs: the code-guidelines HashMap-order section cites line numbers and counts that no longer exist

Open Beginner friendly
#1,706 0 comments 0 reactions 0 assignees View on GitHub
priority:low status:ready type:docs
Dominant language
Rust
Stars
467
Forks
54
Avg merge
4h 25m
Merged PRs (30d)
310

Description

## Summary

The HashMap-iteration-order section of `docs/code-guidelines.md`, which is the written justification for not having a CI gate on this class of bug, cites line numbers, key types, and a file count that no longer match the tree.

## Background

The section's argument rests on concrete evidence (specific eviction sites and their key types) that has since changed: two of the four historical sites were converted to explicit total orders, and the two stores it names now use a `BTreeSet` index. The stale references undercut an otherwise sound argument.

## Proposed Solution

Re-run the documented greps, update the line refs and counts, and reword the paragraph so it reflects the conversions, which strengthen rather than weaken the no-CI-gate argument.

## Implementation Notes

Claimed at `docs/code-guidelines.md:195-197`:

- "run over all 1,248 `.rs` files under `src/`" - the actual count is 1551 (confirmed).
- The four LRU eviction sites with `std::time::Instant` keys "and the tie is not reachable today".

Actual tree:

- `src/server/prompt_cache/store.rs` now has its `min_by_key`/`max_by_key` sites keyed on `(slot.entry.last_used(), *digest.as_bytes())`, an explicit total order with a digest tie-break.
- `responses_store.rs` and `conversation_store.rs` have zero such sites; both use a `BTreeSet` index over the shared `LruKey` in `src/server/store_budget.rs:59-76`.

## Acceptance Criteria

- [ ] Every file:line and count in the section matches the current tree.
- [ ] The paragraph reflects the BTreeSet conversion of the two stores.

---

## Original Suggestion

### Title: docs: the code-guidelines HashMap-order section cites line numbers and counts that no longer exist

The HashMap-iteration-order section of `docs/code-guidelines.md` — the written justification for not having a CI gate on this class of bug — cites line numbers, key types, and a file count that no longer match the tree.

## Evidence

Claimed at `docs/code-guidelines.md:195-197`:

- "run over all 1,248 `.rs` files under `src/`" — actual count is 1551
- "the LRU eviction sites at `src/server/prompt_cache/store.rs:315` and `:336`, `src/server/responses_store.rs:243`, and `src/server/conversation_store.rs:151` … all four keys are `std::time::Instant` … the tie is not reachable today"

Actual tree:

- `src/server/prompt_cache/store.rs:356` and `:381` are the only remaining `min_by_key`/`max_by_key` sites in those files, and their key is `(slot.entry.last_used(), *digest.as_bytes())` — an explicit total order with a digest tie-break (comments at `:350-352`, `:375-377`)
- `responses_store.rs` and `conversation_store.rs` have zero such sites; both now use a `BTreeSet` index over the shared `LruKey` in `src/server/store_budget.rs:59-76`

## Suggested fix

Re-run the documented greps, update the line refs and counts, and reword the paragraph: two of the four historical sites were since converted to explicit total orders, which strengthens (not weakens) the no-CI-gate argument.

## Acceptance criteria

- [ ] Every file:line and count in the section matches the current tree
- [ ] The paragraph reflects the BTreeSet conversion of the two stores

Contributor guide

Open the contributing guide

Research direction

Start with docs/code-guidelines.md:195-197 and rerun the documented greps against the current src/ tree. Verify the Rust references in src/server/prompt_cache/store.rs and the BTreeSet implementation in src/server/store_budget.rs, then update every file:line and count and describe the two converted stores. Done means all references match the tree and the paragraph reflects the current evidence.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
documentation
Issue type
Documentation
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
88/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.