fix: Use raw string values instead of repr() for str types in display_string

[kgilpin]

• Replace isinstance(val, str) with issubclass(type(val), str) to avoid __class__ lookups on proxy/lazy objects
• Update the display_string docstring/comment to reflect that str types return raw values (not repr())
• Verify all tests pass

[Copilot]

Pull request overview

Adjusts AppMap parameter/return-value rendering so str values are recorded as their raw string content (not repr()-quoted), improving consistency with log output and enabling reliable secret-in-log substring matching.

Changes:

• Update display_string to return raw str values instead of repr(val) for strings.
• Update unit tests to assert raw string values for parameters/return values.
• Update JSON “expected” AppMap fixtures to match the new string formatting.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
_appmap/event.py	Changes display_string to emit raw str values instead of repr() output.
_appmap/test/web_framework.py	Updates web framework capture assertions to expect raw string values.
_appmap/test/test_params.py	Updates parameter-capture assertions/parameterized expectations for str values.
_appmap/test/test_http.py	Updates HTTP client capture assertions for raw string query param values.
_appmap/test/test_events.py	Updates labeled params/return value assertions and comment to reflect raw strings.
_appmap/test/data/unittest/expected/unittest.appmap.json	Updates expected recorded parameter/return str values (and formatting) to raw strings.
_appmap/test/data/unittest/expected/unittest-no-test-cases.appmap.json	Updates expected recorded str values to raw strings.
_appmap/test/data/unittest/expected/pytest.appmap.json	Updates expected recorded str values to raw strings.
_appmap/test/data/pytest/expected/pytest-numpy2.appmap.json	Updates expected recorded str return values to raw strings.
_appmap/test/data/pytest/expected/pytest-numpy2-no-test-cases.appmap.json	Updates expected recorded str return values to raw strings.
_appmap/test/data/pytest/expected/pytest-numpy1.appmap.json	Updates expected recorded str return values to raw strings.
_appmap/test/data/pytest/expected/pytest-numpy1-no-test-cases.appmap.json	Updates expected recorded str return values to raw strings.
_appmap/test/data/expected.appmap.json	Updates expected recorded str parameter/return values (including newline escaping) to raw strings.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The comment above still states that parameter display uses repr() to compute a string value, but the implementation now returns the raw value for str types. Please update the comment to reflect the new behavior so it doesn't mislead future readers.

[kgilpin]

@copilot apply changes based on this feedback

[Copilot]

Updated the comment in commit 622ec72 to clarify that str types are returned as-is and other types use repr().

[dividedmind]

Looks good to me. We went back and forth on whether strings should be represented in maps verbatim or as a representation; the scanner usage does finally provide a good argument on one over the other.

The test is also a good addition.

Before merging, please:

• fixup the issubclass and comment fix into the main feature commit,
• amend the main feature commit message to include BREAKING CHANGE: note; this changes the format of the stringified parameters.

One thing I wonder: since we're special casing strings, does it make sense to always capture them, regardless of appmap_display_params value? If it's a string, there is should be no risk of extra serialization overhead. Two potential pitfalls: 1) making sure the class is also always captured in this case, 2) do we want to truncate to avoid putting huge strings in the appmap?

[dividedmind]

Hey @kgilpin, can you take a final look and rebase merge if it looks right?

Re unconditional capture, I think it's a good idea (do you agree?) but best left to a separate PR.

[kgilpin]

@dividedmind 1) What do you mean by "unconditional capture" 2) I don't think we want to make this a new major version release; it is incompatible, but it's also a fix. I am worried about people not picking up this update because they are pinned to 1.x.

[dividedmind]

@dividedmind 1) What do you mean by "unconditional capture"

My suggestions above to always capture strings, regardless of DISPLAY_PARAMS setting. As far as I understand, this setting was introduced to avoid performance overhead related to stringifying complex objects and to avoid potential side effects that stringifying can cause in some (arguably badly designed) third-party objects. Neither of these is a concern when dealing with native scalar types, such as strings, but also eg. numbers, booleans and byte buffers/strings.

2. I don't think we want to make this a new major version release; it is incompatible, but it's also a fix. I am worried about people not picking up this update because they are pinned to 1.x.

It's not really a fix IMO — the format was deliberately chosen as repr-compatible before, so it was working as intended. I'm not too worried about people having pinned 1.x, I don't think we're recommending that anywhere and in any case it's I think relatively uncommon to pin a tool like that in python ecosystem — you either have no constraints or a specific version pin. I'd rather follow semantic versioning to the letter here, I don't think there's any real risk that it will trip someone up and in any case it's easy for them to update if it does.

[kgilpin]

Regarding auto capture, strings can be large so I think the behavior as we have it now in this PR is good (for now).

[dividedmind]

Regarding auto capture, strings can be large so I think the behavior as we have it now in this PR is good (for now).

Yeah, I think the spec allows truncation and I believe some other recording libraries do that; I have actually played with this a bit here so I might push a PR later for discussion.