Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .agents/skills/testing-doc-pdf/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ description: How to end-to-end test the sysml PDF document backend (internal/doc
- Diagram forms: render the worked example with `-diagram-form dot`, `-diagram-form plantuml` and `-diagram-form d2` too; the PDF must contain no "did not draw" notice (pypdf `extract_text`) and the page's XObjects grow by one per diagram. With `OPENSYSML_DOT=/nonexistent` the render still succeeds and the notice names `OPENSYSML_DOT`; the same with `OPENSYSML_D2`. A `// layout: neato -n` block (the DOT writer emits one for a positioned diagram) must run neato: `dot -Kneato` failing with "no layout engine support" means the GTS library is missing.

## Rendering
- Worked example: `bin/sysml docs/manual/examples/observatory.sysml -render-document Observatory::MassReport -doc-form pdf -pdf-title-page -pdf-toc -pdf-number-sections -o /tmp/observatory.pdf`. It exercises emphasis, code span, external Link, Ref (`#breakdown`), a standalone `<a id="breakdown"></a>` anchor line, grouped table (`**zone: ...**` headings), numbered list and two mermaid diagrams.
- Worked example: `bin/sysml docs/manual/examples/observatory.sysml -render-document Observatory::MassReport -doc-form pdf -doc-title-page -doc-toc -doc-number-sections -o /tmp/observatory.pdf`. It exercises emphasis, code span, external Link, Ref (`#breakdown`), a standalone `<a id="breakdown"></a>` anchor line, grouped table (`**zone: ...**` headings), numbered list and two mermaid diagrams.
- Richest input: `internal/doc/docrender/testdata/telescope_report.sysml` → `Observatory::MassReport` covers every inline construct including escaped `\*`/`\|`/`` \` `` prose and reference links; `internal/doc/docpdf/fixture_test.go` evaluates it into a `*docir.Document`, which is what `docpdf.Render(document, "weasyprint", docpdf.Options{...})` takes (not Markdown).
- Markdown regression: `-o foo.md` must diff-equal `docs/manual/examples/observatory.md`. Note `-doc-form` takes `markdown`, `html` or `pdf` (not `md`).
- WeasyPrint and Prince read the HTML backend's page (`docrender.HTML`, the same markup as `-doc-form html`) under `internal/doc/docpdf/print.css`, so `-html-theme`, `-html-css` and `-html-no-default-css` change the PDF; pandoc reads the Markdown and refuses the theme flags with a typed error. Inspect the prepared input with `docpdf.Render`'s fake-tool tests (`TestRenderHTMLIsTheBackendsPage`) or a fake `OPENSYSML_WEASYPRINT` script that copies its input file aside.
Expand Down
29 changes: 14 additions & 15 deletions .agents/skills/testing-sysml-repl/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -1831,7 +1831,7 @@ to A/B the two surfaces on the same model. The call shapes are **not** the ones
suggest: there is no `parse_file` on `Connection` — use `c.load_from_content(src)` (or `c.load(path)`),
which returns a `Model` whose hash is `model.hash` (not `.model_hash`). Then
`c.execute_action("Pkg::action", model.hash)` returns a plain `{name: value}` dict, and a runtime
failure raises `opensysml.errors.RuntimeError` with the executor's message
failure raises `opensysml.ExecutionError` with the executor's message
(`action execution failed: execute action: …`). If you must attach to a service you started
yourself, `opensysml.connect("localhost", 50551, auto_start=False)` avoids the
`Binary not found at ~/.opensysml/bin/sysml-grpc` error — but prefer the auto-start path per above.
Expand Down Expand Up @@ -3447,8 +3447,8 @@ number rather than quoting the table.
`m.eval('mass', subject='Demo::sedan')` must be `1200.0`. Without the redefinition both paths
return the same number and the test proves nothing.
- On `Model` the kwarg is `subject=`; on `Connection.eval` it is `subject_symbol_id=` and the
model hash comes **second**. Module-level `opensysml.evaluate` also takes `subject=`; the
deprecated `opensysml.eval` forwards to it and warns `DeprecationWarning`.
model hash comes **second**. Module-level `opensysml.evaluate` also takes `subject=` (the
`opensysml.eval` alias was removed in 0.12.0).
- A `subject=` plus an explicit `context_symbol_id=` still reads the object, and derived slots
(`attribute doubled = mass * 2.0`) follow the object too.
- A *definition* as subject is accepted (it is instantiated and reads its own redefinition), and an
Expand Down Expand Up @@ -3951,19 +3951,18 @@ tests skip rather than fail.

## The evaluate/eval split and generated-base planning (PR #218)

- **Since the rename, `opensysml.evaluate` is the real module-level evaluator and `opensysml.eval` is a
forwarder that emits `DeprecationWarning` and is out of `opensysml.__all__`.** Test both sides:
`evaluate(...)` must produce **zero** DeprecationWarnings (catch them with
`warnings.catch_warnings(record=True)` + `simplefilter("always")`), `eval(...)` must return the
identical value and warn exactly once with a message naming `opensysml.evaluate`, and
`from opensysml import *` must bind `evaluate` but not `eval`. `subject` is the **last** parameter of
`evaluate` precisely so a pre-rename positional call
`eval(expr, None, hash, None, host, port)` still binds host/port — prove that argument really is the
host by also calling it with a bogus address (`"203.0.113.9", 59999`) and requiring a
- **`opensysml.evaluate` is the module-level evaluator; the `opensysml.eval` forwarder and the
`opensysml.RuntimeError` / `opensysml.errors.RuntimeError` aliases were removed in 0.12.0** (they
warned `DeprecationWarning` from 0.9.0). Test that `evaluate(...)` produces **zero** warnings (catch
them with `warnings.catch_warnings(record=True)` + `simplefilter("always")`), that
`from opensysml import *` binds `evaluate` and `ExecutionError` but neither `eval` nor
`RuntimeError`, and that `opensysml.eval` / `opensysml.errors.RuntimeError` raise `AttributeError`.
`subject` is the **last** parameter of `evaluate`, so a pre-rename positional call
`evaluate(expr, None, hash, None, host, port)` still binds host/port — prove that argument really is
the host by also calling it with a bogus address (`"203.0.113.9", 59999`) and requiring a
`ConnectionError`; that call takes ~30-60s to time out, so give the runner a generous timeout.
- **`opensysml.errors.RuntimeError` is a warn-on-access alias of `ExecutionError`** served by the module
`__getattr__` and absent from `errors.__all__`. Check it by *catching a real failure* with it
(`except opensysml.errors.RuntimeError` around a cyclic-slot eval), not just by identity.
- **`opensysml.ExecutionError` is also a built-in `RuntimeError`**, so `except RuntimeError` catches a
real failure (a cyclic-slot eval) — check it by catching, not just by identity.
- **`generate.py` elides a base that another declared base already specializes, silently and by design**
(`_without_implied`): `part def Backwards :> Vehicle, Hybrid` where `Hybrid :> Vehicle` emits
`class Backwards(Hybrid):` with **no** comment, because `Vehicle` is still in the MRO. Only a base the
Expand Down
1 change: 1 addition & 0 deletions changes/unreleased/cli-former-flag-names.removed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- The `sysml` CLI's former flag names: `-to`, replaced by `-convert` in 0.0.7, and `-pdf-title-page`, `-pdf-toc` and `-pdf-number-sections`, replaced by `-doc-title-page`, `-doc-toc` and `-doc-number-sections` in 0.9.0. Only the replacements are declared.
1 change: 1 addition & 0 deletions changes/unreleased/go-edit-result-content.removed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- The Go client's `EditResult.Content`, deprecated in 0.9.0 in favour of `EditResult.Documents`, which carries the edited notation of a model of one document as well as of several. `ApplyEdits` and `ApplyDocumentEdits` now require the `edit_documents` capability every service since 0.9.0 advertises, and are refused with `CodeUnimplemented` by one without it.
1 change: 1 addition & 0 deletions changes/unreleased/node-strict-alias.removed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- The Node client's deprecated `strict` parse option, an alias of `strictConformance` since 0.10.0; pass `strictConformance`.
1 change: 1 addition & 0 deletions changes/unreleased/python-deprecated-aliases.removed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- The Python client's deprecated aliases `opensysml.eval`, `opensysml.RuntimeError` and `opensysml.errors.RuntimeError`, which warned and answered `opensysml.evaluate` and `opensysml.ExecutionError` since 0.9.0; write the replacements.
4 changes: 2 additions & 2 deletions client/node/conformance/runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -513,7 +513,7 @@ export class Runner {
private async parseFile(request: Record<string, unknown>): Promise<void> {
const options = {
...stringOption(request, "language", "language"),
...flagOption(request, "strict_conformance", "strict"),
...flagOption(request, "strict_conformance", "strictConformance"),
};
const content = request["content"];
if (typeof content === "string") {
Expand Down Expand Up @@ -577,7 +577,7 @@ export class Runner {
model.fixtures === undefined
? await this.connection.loads(this.fixture(model.fixture ?? ""), {
...(model.language === undefined ? {} : { language: model.language }),
...(model.strict_conformance === undefined ? {} : { strict: model.strict_conformance }),
...(model.strict_conformance === undefined ? {} : { strictConformance: model.strict_conformance }),
})
: await this.connection.parseSources(
model.fixtures.map((name) => SourceDocument.inline(name, this.fixture(name))),
Expand Down
3 changes: 1 addition & 2 deletions client/node/src/core/connection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,6 @@ import {
decodeDiagnostic,
Instance,
Model,
strictConformanceOf,
type ParseOptions,
type ParseSourcesOptions,
} from "./model.js";
Expand Down Expand Up @@ -390,7 +389,7 @@ export class Connection {
);
capabilities.push(CAPABILITY_INLINE_LANGUAGE);
}
const strictConformance = strictConformanceOf(options);
const { strictConformance } = options;
if (strictConformance === true) {
this.requireStrictConformance();
capabilities.push(CAPABILITY_STRICT_CONFORMANCE);
Expand Down
2 changes: 1 addition & 1 deletion client/node/src/core/conversion.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// Writing a model back out, in SysML notation or RDF Turtle. The formats are
// named as the `sysml` CLI's -from/-to name them.
// named as the `sysml` CLI's -from/-convert name them.

import { byCodeUnit } from "./capabilities.js";
import type { ModelDiagnostic } from "./errors.js";
Expand Down
25 changes: 1 addition & 24 deletions client/node/src/core/model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,11 +82,6 @@ export interface ParseOptions {
language?: string;
/** Reject the OpenSysML notation extensions; requires `strict_conformance`. */
strictConformance?: boolean;
/**
* @deprecated Use `strictConformance`; `strict` is its alias. Passing both
* with different values is refused.
*/
strict?: boolean;
}

/** How several documents are parsed as one model. */
Expand All @@ -100,24 +95,6 @@ export interface ParseSourcesOptions extends ParseOptions {
baseModelHash?: string;
}

/** Resolves `strictConformance` and its `strict` alias; differing values refuse. */
export function strictConformanceOf(options: {
strict?: boolean;
strictConformance?: boolean;
}): boolean | undefined {
if (
options.strict !== undefined &&
options.strictConformance !== undefined &&
options.strict !== options.strictConformance
) {
throw new RangeError(
`strict (${options.strict}) and strictConformance (${options.strictConformance}) disagree; ` +
"pass only strictConformance",
);
}
return options.strictConformance ?? options.strict;
}

/** Where an expression is evaluated. */
export interface EvalOptions {
/** FQN of the symbol whose scope the expression's names resolve in. */
Expand Down Expand Up @@ -301,7 +278,7 @@ export class Model {
requireLanguage(options.language);
requireCapability(connection.info, CAPABILITY_INLINE_LANGUAGE, upgradeRemedy(CAPABILITY_INLINE_LANGUAGE));
}
const strictConformance = strictConformanceOf(options);
const { strictConformance } = options;
Comment thread
devin-ai-integration[bot] marked this conversation as resolved.
Resolved
if (strictConformance === true) {
requireCapability(
connection.info,
Expand Down
33 changes: 22 additions & 11 deletions client/node/test/sources.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,19 @@ const BROKEN = `package Broken {
}
`;

// An OpenSysML notation extension (`choice`), a warning by default and an error under strict conformance.
const EXTENDED = `package P {
state def S {
entry; then a;
state a;
state b;
choice c;
transition a_c first a then c;
transition c_b first c then b;
}
}
`;

test("several documents parse as one model resolving imports across them", async () => {
await using connection = await connect();
const model = await connection.parseSources([
Expand Down Expand Up @@ -165,18 +178,16 @@ test("a broken document reports diagnostics naming it", async () => {
assert.ok(model.diagnostics.every((diagnostic) => diagnostic.file === "broken"));
});

test("strict is a deprecated alias of strictConformance; disagreeing values refuse", async () => {
test("strictConformance makes an OpenSysML-only notation the error the default parse reports as a warning", async () => {
await using connection = await connect();
await assert.rejects(
() => connection.parseSources([["top", TOP]], { strict: true, strictConformance: false }),
RangeError,
);
await assert.rejects(
() => connection.loads(TOP, { strict: false, strictConformance: true }),
RangeError,
);
const model = await connection.parseSources([["broken", BROKEN]], { strict: true });
assert.throws(() => model.raiseForErrors(), ParseError);
const lenient = await connection.parseSources([["extended", EXTENDED]]);
assert.equal(lenient.errors.length, 0);
assert.ok(lenient.diagnostics.some((diagnostic) => diagnostic.severity === "warning"));
const strict = await connection.parseSources([["extended", EXTENDED]], { strictConformance: true });
assert.ok(strict.errors.length > 0);
assert.throws(() => strict.raiseForErrors(), ParseError);
const loaded = await connection.loads(EXTENDED, { strictConformance: true });
assert.ok(loaded.errors.length > 0);
});

test("a document whose name another takes is refused before it is sent", async () => {
Expand Down
27 changes: 13 additions & 14 deletions client/opensysml/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -316,10 +316,8 @@ a new or an existing declaration and require `authoring` and
`authoring` and `comment_authoring`.

The edited source is `result.Documents`, one `EditedDocument` per document the
batch reached, under the name the model was parsed with; `result.Content` is the
same notation for a model of one document and empty for a model of several, kept
for callers of the sole-document contract. Each `AppliedEdit` names the
`Document` its bytes belong to.
batch reached, under the name the model was parsed with — for a model of one
document, that one. Each `AppliedEdit` names the `Document` its bytes belong to.

## What a model is here

Expand Down Expand Up @@ -385,16 +383,17 @@ another, and a name that is not one of the model's is `CodeInvalidArgument`. A
rename or a cascade delete follows its references into the other documents, every
document touched is re-parsed and re-analysed together, and `result.Documents`
lists exactly the documents rewritten — so a batch that reaches one document of
three answers one `EditedDocument`, and `result.Content` is empty. The client marks
every request as accepting documents; the service refuses a request that does not on a
model of several, as it did before, so a program reading `Content` alone through an
earlier client is never handed an empty one. A reference from
a document the edit cannot rewrite, such as a bundled library file, refuses the
edit as `EditFailureReferencedElsewhere`, naming it in `Referrers`. All of this is the
`CapabilityEditDocuments` capability: a service without it edits a model of one
document alone, answering `Content` with `Documents` empty, refuses a model of several
with `CodeFailedPrecondition`, and refuses `ApplyDocumentEdits` with `CodeUnimplemented`
— so a program reading `Documents` checks `ServerInfo` for the capability first.
three answers one `EditedDocument`. The client marks every request as accepting
documents; the service refuses a request that does not on a model of several, as
it did before, so a program reading the sole-document `content` through an earlier
client is never handed an empty one. A reference from a document the edit cannot
rewrite, such as a bundled library file, refuses the edit as
`EditFailureReferencedElsewhere`, naming it in `Referrers`. All of this is the
`CapabilityEditDocuments` capability, which every service since 0.9.0 advertises:
`ApplyEdits` and `ApplyDocumentEdits` require it and are refused with
`CodeUnimplemented` by a service without it, whose answer carried the notation in
the sole-document `content` alone — the `EditResult.Content` field that read it,
deprecated in 0.9.0, was removed in 0.12.0.

## Concurrency, contexts and lifetime

Expand Down
42 changes: 5 additions & 37 deletions client/opensysml/authoring.go
Original file line number Diff line number Diff line change
Expand Up @@ -786,19 +786,10 @@ func (f EditFailure) String() string {

// EditResult is a model's source with every edit applied.
type EditResult struct {
// Content is the edited notation of a single-document model, byte-identical
// to the source outside the edited spans. Empty for a model of several
// documents, whose notation is in Documents alone.
//
// Deprecated: read Documents, which carries the same notation for a model of
// one document and every rewritten document of a model of several, from a
// service advertising the edit_documents capability.
Content string
// Documents is the edited notation of every document the edits rewrote: the
// edited document first when it is among them, then the others in name order.
// A single-document model's one document is always listed; a document of
// several the edits left as parsed is not. Empty, with Content filled, from a
// service without the edit_documents capability, which edits one document alone.
// several the edits left as parsed is not.
Documents []EditedDocument
// Applied says what each edit changed, grouped by document in the order
// Documents lists them and in request order within a document.
Expand Down Expand Up @@ -846,25 +837,6 @@ func (c *client) ApplyEdits(ctx context.Context, model *Model, edits ...Edit) (*
return c.ApplyDocumentEdits(ctx, model, "", edits...)
}

// requireEditDocuments refuses to send a document name to a service without
// the edit_documents capability, which would ignore it and edit its sole document.
func (c *client) requireEditDocuments(ctx context.Context, document string) error {
if document == "" {
return nil
}
info, err := c.serverInfo(ctx)
if err != nil {
return err
}
if !info.Has(CapabilityEditDocuments) {
return &StatusError{
Code: CodeUnimplemented,
Message: fmt.Sprintf("capability %q is unavailable", CapabilityEditDocuments),
}
}
return nil
}

func (c *client) requireCapabilities(ctx context.Context, capabilities ...string) error {
info, err := c.serverInfo(ctx)
if err != nil {
Expand All @@ -886,15 +858,11 @@ func (c *client) ApplyDocumentEdits(ctx context.Context, model *Model, document
if err != nil {
return nil, err
}
if err := c.requireEditDocuments(ctx, document); err != nil {
// The notation is read from Documents, which only a service with the
// edit_documents capability answers (one without it ignores a document name).
if err := c.requireCapabilities(ctx, append(requiredCapabilities(edits), CapabilityEditDocuments)...); err != nil {
return nil, err
}
if names := requiredCapabilities(edits); len(names) > 0 {
if err := c.requireCapabilities(ctx, names...); err != nil {
return nil, err
}
}
// This client reads Documents, so a model of several documents may be edited.
req := &pb.ApplyEditsRequest{ModelHash: hash, Document: document, AcceptDocuments: true}
for _, edit := range edits {
operation, err := editToProto(edit)
Expand All @@ -916,7 +884,7 @@ func (c *client) ApplyDocumentEdits(ctx context.Context, model *Model, document
Referrers: referrersFromProto(resp.Referrers),
}
}
result := &EditResult{Content: resp.Content, Diagnostics: diagnostics}
result := &EditResult{Diagnostics: diagnostics}
for _, doc := range resp.Documents {
result.Documents = append(result.Documents, EditedDocument{Name: doc.Name, Content: doc.Content})
}
Expand Down
5 changes: 2 additions & 3 deletions client/opensysml/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -232,9 +232,8 @@ type Client interface {
// them all with an EditError. The edits target the model's first document —
// its only one when parsed from a file or source — and a rename or cascade
// delete follows references into every other document of the model, whose
// rewritten notation the result lists. Requires the apply_edits capability;
// a model of several documents, and a result listing Documents, the
// edit_documents capability too.
// rewritten notation the result lists. Requires the apply_edits and
// edit_documents capabilities.
ApplyEdits(ctx context.Context, model *Model, edits ...Edit) (*EditResult, error)

// ApplyDocumentEdits is ApplyEdits with the edits targeting the document
Expand Down
Loading
Loading