Skip to content

Commit bea4592

Browse files
authored
Merge pull request #439 from control-toolbox/feature/private-public-methods
- Add naming conventions to modules.md: symbol (public), _symbol (private helper), __symbol (default value) - Rename private functions in Descriptions: compute_similarity → _compute_similarity, find_similar_descriptions → _find_similar_descriptions, format_description_candidates → _format_description_candidates - Rename private functions in Exceptions: extract_user_frames → _extract_user_frames, format_user_friendly_error → _format_user_friendly_error, ansi_color → _ansi_color, ansi_reset → _ansi_reset, print_ansi_styled → _print_ansi_styled - Update planning.md with improved checkpoint templates (MCP preferred, fallback commands) - Fix extension tests to use fake types per testing-creation.md §6 (prevents flaky tests when extensions are loaded elsewhere)
2 parents a2c5d45 + 4e6555c commit bea4592

9 files changed

Lines changed: 185 additions & 106 deletions

File tree

dev/philosophy/modules.md

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,9 +92,31 @@ end
9292
Why: visible origin, safe refactors (only the `using ..X` line changes), no accidental
9393
shadowing, same rule for external and sibling symbols.
9494

95+
## Naming conventions
96+
97+
Three visibility levels, signalled by the name itself:
98+
99+
- `symbol`**public**: exported via `export`; part of the documented API.
100+
- `_symbol`**private helper**: unexported; internal implementation detail.
101+
Accessible via qualified path (`Module._symbol`) but not advertised.
102+
- `__symbol`**default value**: unexported; provides a replaceable semantic
103+
default (e.g. `__display() = true`). The double underscore distinguishes it
104+
from a plain helper: it is a *default* that a higher-level layer may override,
105+
not merely a utility function.
106+
107+
```julia
108+
export build_thing # public
109+
110+
function _validate(x) # private helper
111+
...
112+
end
113+
114+
__verbose()::Bool = false # default value (replaceable by extension or subtype)
115+
```
116+
95117
## Two-level exports
96118

97-
- **Submodule**: `export` for its public API; internals (`_helper`) unexported.
119+
- **Submodule**: `export` for its public API; internals (`_helper`, `__default`) unexported.
98120
- **Package (top-level)**: **no** `export`. Load submodules with `using .Submodule`;
99121
users reach them via `Package.Submodule.sym`.
100122

dev/planning.md

Lines changed: 83 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ to align with the human before touching code, not to produce a 50-step document.
66
## When to write a plan
77

88
Before any task that:
9+
910
- touches more than one file,
1011
- changes a public interface, or
1112
- involves a non-obvious design decision.
@@ -16,7 +17,7 @@ Write the plan, share it, wait for confirmation before starting.
1617

1718
## Template
1819

19-
```
20+
````markdown
2021
# Plan: <short title>
2122

2223
## What and why
@@ -34,32 +35,74 @@ Write the plan, share it, wait for confirmation before starting.
3435
Steps:
3536
1.
3637
2.
37-
Checkpoint: run `test/suite/<group>` via MCP. All green before moving on.
38+
Checkpoint:
39+
40+
```bash
41+
# MCP (preferred)
42+
get_test_command test_args=["suite/<group>"]
43+
then generate_report
44+
45+
# Fallback
46+
julia --project=@. -e 'using Pkg; Pkg.test(; test_args=["suite/<group>"])' 2>&1 | tee /tmp/<branch>_phaseA.log
47+
grep -E "Error|Fail|Test Summary" /tmp/<branch>_phaseA.log
48+
```
49+
50+
All green before moving on.
3851

3952
### Phase B — <name>
53+
4054
Steps:
4155
1.
42-
Checkpoint: run `test/suite/<group>` + full suite.
56+
Checkpoint:
57+
58+
```bash
59+
# MCP (preferred)
60+
get_test_command test_args=["suite/<group1>", "suite/<group2>"]
61+
then generate_report
62+
63+
# Final phase only: full suite
64+
julia --project=@. -e 'using Pkg; Pkg.test()' 2>&1 | tee /tmp/<branch>_final.log
65+
grep -E "Error|Fail|Test Summary" /tmp/<branch>_final.log
66+
```
4367

4468
4569

4670
## Human checkpoints
71+
4772
- ⛔ Ask before first commit.
4873
- ⛔ Ask before pushing to a shared branch.
4974
- ⛔ Ask before deleting files that were not created in this task.
5075
- ⛔ Ask if a design decision arises that was not in the plan.
5176

5277
## Out of scope
78+
5379
<List things explicitly not done in this task.>
54-
```
80+
````
5581

5682
---
5783

5884
## Rules for filling in the template
5985

6086
**Phases** — one phase = one coherent unit that can be tested independently. A phase
61-
ends with a test checkpoint (run the affected test suite via the MCP `get_test_command`
62-
tool, then `generate_report`). Never skip the checkpoint.
87+
ends with a test checkpoint. Never skip the checkpoint.
88+
89+
**Test checkpoints** — always use the MCP `get_test_command` tool with `test_args` to
90+
run only the affected test groups, then `generate_report` to parse the result. If MCP
91+
is unavailable, fall back to:
92+
93+
```bash
94+
# Phase-specific (preferred during intermediate phases)
95+
julia --project=@. -e 'using Pkg; Pkg.test(; test_args=["suite/<group>"])' \
96+
2>&1 | tee /tmp/<branch>_<phase>.log
97+
grep -E "Error|Fail|Test Summary" /tmp/<branch>_<phase>.log
98+
99+
# Full suite (mandatory for the last phase only)
100+
julia --project=@. -e 'using Pkg; Pkg.test()' \
101+
2>&1 | tee /tmp/<branch>_final.log
102+
grep -E "Error|Fail|Test Summary" /tmp/<branch>_final.log
103+
```
104+
105+
Never run the full suite for intermediate phases — it is too slow and hides errors.
63106

64107
**Steps** — file-level granularity. "Edit `src/X/y.jl` to add method `f`" not "implement
65108
the feature". Concrete enough that a step is either done or not.
@@ -70,14 +113,23 @@ missed.
70113

71114
**Docstrings** — always the last step of the last phase, once the API is stable.
72115

116+
**Code snippets** — include short code snippets directly in the plan to guide the
117+
implementer. This applies to:
118+
119+
- *Source changes*: show the before/after signature or struct definition.
120+
- *Call-site updates*: show the exact old and new lines when renaming symbols.
121+
- *Test patterns*: show the test structure (fake type + testset skeleton) when the
122+
test pattern is non-obvious.
123+
Snippets should be *concise shapes*, not full implementations.
124+
73125
**Out of scope** — explicit. Prevents scope creep and clarifies what the human should
74126
not expect after the plan is executed.
75127

76128
---
77129

78130
## Concrete example structure (abbreviated)
79131

80-
```
132+
````markdown
81133
# Plan: rename content → dynamics trait
82134

83135
## What and why
@@ -97,23 +149,43 @@ names the axis. Purely mechanical, no behavior change.
97149
Steps:
98150
1. Rename src/Traits/content.jl → dynamics.jl; update all names inside.
99151
2. Update include path in src/Traits/Traits.jl; update exports.
100-
Checkpoint: test/suite/traits — all green.
152+
Checkpoint:
153+
154+
```bash
155+
get_test_command test_args=["suite/traits"] # MCP
156+
# fallback: julia --project=@. -e 'using Pkg; Pkg.test(; test_args=["suite/traits"])' 2>&1 | tee /tmp/rename_traits.log
157+
```
101158

102159
### Phase B — Adapt consumers (Configs, Solutions)
160+
103161
Steps:
162+
104163
1. Update src/Configs/* (content_trait → dynamics_trait, value names).
105164
2. Update src/Solutions/building.jl.
106-
Checkpoint: test/suite/configs + test/suite/solutions — all green.
165+
Checkpoint:
166+
167+
```bash
168+
get_test_command test_args=["suite/configs", "suite/solutions"] # MCP
169+
```
107170

108171
### Phase C — Tests + docs
172+
109173
Steps:
174+
110175
1. Rename and update test/suite/traits/test_content.jl → test_dynamics.jl.
111176
2. Update docs/api_reference.jl file list.
112-
Checkpoint: full suite green.
177+
Checkpoint (full suite — last phase):
178+
179+
```bash
180+
get_test_command # MCP, no test_args = full suite
181+
# fallback: julia --project=@. -e 'using Pkg; Pkg.test()' 2>&1 | tee /tmp/rename_final.log
182+
```
113183

114184
## Human checkpoints
185+
115186
- ⛔ Ask before commit after Phase C.
116187

117188
## Out of scope
189+
118190
Parametrizing AbstractSystem/AbstractFlow by the dynamics trait (see action_plan.md Phase C/D).
119-
```
191+
````

src/Descriptions/complete.jl

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ ERROR: AmbiguousDescription: the description (:f,) is ambiguous / incorrect
4545
Context: description completion
4646
```
4747
48-
See also: [`CTBase.Descriptions.compute_similarity`](@ref), [`CTBase.Descriptions.find_similar_descriptions`](@ref), [`CTBase.Descriptions.format_description_candidates`](@ref), [`CTBase.Exceptions.AmbiguousDescription`](@ref)
48+
See also: [`CTBase.Descriptions._compute_similarity`](@ref), [`CTBase.Descriptions._find_similar_descriptions`](@ref), [`CTBase.Descriptions._format_description_candidates`](@ref), [`CTBase.Exceptions.AmbiguousDescription`](@ref)
4949
"""
5050
function complete(list::Symbol...; descriptions::Tuple{Vararg{Description}})::Description
5151
n = length(descriptions)
@@ -70,8 +70,8 @@ function complete(list::Symbol...; descriptions::Tuple{Vararg{Description}})::De
7070

7171
if maximum(table[:, 2]) == 0
7272
# Find similar descriptions for helpful suggestions
73-
similar_descs = find_similar_descriptions(list, descriptions; max_results=5)
74-
all_candidates = format_description_candidates(descriptions; max_show=10)
73+
similar_descs = _find_similar_descriptions(list, descriptions; max_results=5)
74+
all_candidates = _format_description_candidates(descriptions; max_show=10)
7575

7676
# Build contextual suggestion
7777
suggestion = if !isempty(similar_descs)

src/Descriptions/similarity.jl

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ julia> CTBase.Descriptions.compute_similarity((:x, :y), (:a, :b))
2323
0.0
2424
```
2525
"""
26-
function compute_similarity(desc1::Description, desc2::Description)::Float64
26+
function _compute_similarity(desc1::Description, desc2::Description)::Float64
2727
if isempty(desc1) || isempty(desc2)
2828
return 0.0
2929
end
@@ -62,7 +62,7 @@ julia> CTBase.Descriptions.find_similar_descriptions((:a,), descriptions)
6262
"(:a, :c)"
6363
```
6464
"""
65-
function find_similar_descriptions(
65+
function _find_similar_descriptions(
6666
target::Tuple{Vararg{Symbol}},
6767
descriptions::Tuple{Vararg{Description}};
6868
max_results::Int=5,
@@ -73,7 +73,7 @@ function find_similar_descriptions(
7373

7474
# Compute similarities
7575
similarities = [
76-
(compute_similarity(target, desc), string(desc)) for desc in descriptions
76+
(_compute_similarity(target, desc), string(desc)) for desc in descriptions
7777
]
7878

7979
# Sort by similarity (descending) and take top results
@@ -118,7 +118,7 @@ julia> CTBase.Descriptions.format_description_candidates(descriptions; max_show=
118118
"(:x, :y)"
119119
```
120120
"""
121-
function format_description_candidates(
121+
function _format_description_candidates(
122122
descriptions::Tuple{Vararg{Description}}; max_show::Int=5
123123
)::Vector{String}
124124
if isempty(descriptions)

src/Exceptions/display.jl

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ Filters out Julia stdlib.
4444
# Returns
4545
- `Vector`: Filtered stacktrace frames
4646
"""
47-
function extract_user_frames(st::Vector)
47+
function _extract_user_frames(st::Vector)
4848
user_frames = filter(st) do frame
4949
file_str = string(frame.file)
5050
# Keep frames that are NOT from Julia stdlib or exception display internals
@@ -68,7 +68,7 @@ Display an error in a user-friendly format with clear sections and user code loc
6868
- `io::IO`: Output stream
6969
- `e::CTException`: The exception to display
7070
"""
71-
function format_user_friendly_error(io::IO, e::CTException)
71+
function _format_user_friendly_error(io::IO, e::CTException)
7272
#println(io, "\n" * "━"^70)
7373
_print_ansi_styled(io, "Control Toolbox Error\n", :red, true)
7474
#println(io, "─"^28)
@@ -232,7 +232,7 @@ function format_user_friendly_error(io::IO, e::CTException)
232232
end
233233

234234
# Add user code location
235-
user_frames = extract_user_frames(stacktrace(catch_backtrace()))
235+
user_frames = _extract_user_frames(stacktrace(catch_backtrace()))
236236
if !isempty(user_frames)
237237
println(io, "📍 In your code:")
238238
# Show up to 3 most relevant user frames
@@ -262,7 +262,7 @@ Custom error display for IncorrectArgument.
262262
Shows user-friendly format with enriched information.
263263
"""
264264
function Base.showerror(io::IO, e::IncorrectArgument)
265-
return format_user_friendly_error(io, e)
265+
return _format_user_friendly_error(io, e)
266266
end
267267

268268
"""
@@ -271,7 +271,7 @@ end
271271
Custom error display for PreconditionError.
272272
"""
273273
function Base.showerror(io::IO, e::PreconditionError)
274-
return format_user_friendly_error(io, e)
274+
return _format_user_friendly_error(io, e)
275275
end
276276

277277
"""
@@ -280,7 +280,7 @@ end
280280
Custom error display for NotImplemented.
281281
"""
282282
function Base.showerror(io::IO, e::NotImplemented)
283-
return format_user_friendly_error(io, e)
283+
return _format_user_friendly_error(io, e)
284284
end
285285

286286
"""
@@ -289,7 +289,7 @@ end
289289
Custom error display for ParsingError.
290290
"""
291291
function Base.showerror(io::IO, e::ParsingError)
292-
return format_user_friendly_error(io, e)
292+
return _format_user_friendly_error(io, e)
293293
end
294294

295295
"""
@@ -298,7 +298,7 @@ end
298298
Custom error display for AmbiguousDescription.
299299
"""
300300
function Base.showerror(io::IO, e::AmbiguousDescription)
301-
return format_user_friendly_error(io, e)
301+
return _format_user_friendly_error(io, e)
302302
end
303303

304304
"""
@@ -307,7 +307,7 @@ end
307307
Custom error display for ExtensionError.
308308
"""
309309
function Base.showerror(io::IO, e::ExtensionError)
310-
return format_user_friendly_error(io, e)
310+
return _format_user_friendly_error(io, e)
311311
end
312312

313313
"""
@@ -316,5 +316,5 @@ end
316316
Custom error display for SolverFailure.
317317
"""
318318
function Base.showerror(io::IO, e::SolverFailure)
319-
return format_user_friendly_error(io, e)
319+
return _format_user_friendly_error(io, e)
320320
end

src/Extensions/Extensions.jl

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -332,7 +332,10 @@ function run_tests(; kwargs...)
332332
return run_tests(TestRunnerTag(); kwargs...)
333333
end
334334

335-
# Export public API (only user-facing functions, tags are internal)
335+
# Export public API
336336
export automatic_reference_documentation, postprocess_coverage, run_tests
337+
export AbstractDocumenterReferenceTag, DocumenterReferenceTag
338+
export AbstractCoveragePostprocessingTag, CoveragePostprocessingTag
339+
export AbstractTestRunnerTag, TestRunnerTag
337340

338341
end # module

0 commit comments

Comments
 (0)